Browse Source

[3971] Resolved issues with fork/exec while spawning LFC.

Marcin Siodelski 9 years ago
parent
commit
973e98b726

+ 4 - 1
src/bin/dhcp4/tests/dhcp4_process_tests.sh.in

@@ -18,6 +18,8 @@ CFG_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test_config.json
 LOG_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test.log
 # Path to the Kea lease file.
 LEASE_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test_leases.csv
+# Path to the Kea LFC application
+export KEA_LFC_EXECUTABLE=@abs_top_builddir@/src/bin/lfc/kea-lfc
 # Expected version
 EXPECTED_VERSION="@PACKAGE_VERSION@"
 # Kea configuration to be stored in the configuration file.
@@ -288,7 +290,8 @@ lfc_timer_test() {
     cleanup
     # Create a configuration with the LFC enabled, by replacing the section
     # with the lfc-interval parameter.
-    LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 0/\"lfc-interval\": 1/g')
+    LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 0/\"lfc-interval\": 1/g' \
+                        | sed -e 's/\"persist\": false/\"persist\": true/g')
     echo ${LFC_CONFIG}
     # Create new configuration file.
     create_config "${LFC_CONFIG}"

+ 4 - 3
src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -130,9 +130,10 @@ LFCSetup::LFCSetup(asiolink::IntervalTimer::Callback callback)
 
 LFCSetup::~LFCSetup() {
     try {
-        // If we're here it means that we are reconfiguring the server or
-        // the server is terminating. In both cases the thread must
-        // be already stopped or must stop now.
+        // If we're here it means that either the process is terminating
+        // or we're reconfiguring the server. In the latter case the
+        // thread has been stopped probably, but we need to handle the
+        // former case so we call stopThread explicitly here.
         timer_mgr_->stopThread();
         // This may throw exception if the timer hasn't been registered
         // but if the LFC Setup instance exists it means that the timer

+ 1 - 0
src/lib/util/Makefile.am

@@ -10,6 +10,7 @@ lib_LTLIBRARIES = libkea-util.la
 libkea_util_la_SOURCES  = boost_time_utils.h boost_time_utils.cc
 libkea_util_la_SOURCES += csv_file.h csv_file.cc
 libkea_util_la_SOURCES += filename.h filename.cc
+libkea_util_la_SOURCES += fork_detector.h
 libkea_util_la_SOURCES += strutil.h strutil.cc
 libkea_util_la_SOURCES += buffer.h io_utilities.h
 libkea_util_la_SOURCES += time_utilities.h time_utilities.cc

+ 62 - 0
src/lib/util/fork_detector.h

@@ -0,0 +1,62 @@
+// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef FORK_DETECTOR_H
+#define FORK_DETECTOR_H
+
+#include <sys/types.h>
+#include <unistd.h>
+
+namespace isc {
+namespace util {
+
+/// @brief Class which detects being child process.
+///
+/// This class detects if it is in the child process, by checking if the
+/// process PID matches the PID of the process which created instance of
+/// the class.
+///
+/// Detecting if we're in the child process is important when the application
+/// spawns new process using fork/exec. If exec step fails for any reason
+/// the child process exits. However, to exit gracefully the process may
+/// need to know if it is a child or parent and take a different path
+/// during destruction.
+class ForkDetector {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Stores the PID of the process creating this instance.
+    ForkDetector()
+        : creator_pid_(getpid()) {
+    }
+
+    /// @brief Check if the process is a parent process;
+    ///
+    /// @return true if the process is a parent process.
+    bool isParent() const {
+        return (getpid() == creator_pid_);
+    }
+
+private:
+
+    /// @brief PID of the process which created instance of this class.
+    pid_t creator_pid_;
+
+};
+
+} // namespace isc::util
+} // namespace isc
+
+#endif // FORK_DETECTOR_H

+ 23 - 6
src/lib/util/threads/thread.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <util/fork_detector.h>
 #include <util/threads/thread.h>
 #include <util/threads/sync.h>
 
@@ -74,7 +75,8 @@ public:
         // and the creating thread needs to release it.
         waiting_(2),
         main_(main),
-        exception_(false)
+        exception_(false),
+        fork_detector_()
     {}
     // Another of the waiting events is done. If there are no more, delete
     // impl.
@@ -125,6 +127,8 @@ public:
     Mutex mutex_;
     // Which thread are we talking about anyway?
     pthread_t tid_;
+    // Class to detect if we're in the child or parent process.
+    ForkDetector fork_detector_;
 };
 
 Thread::Thread(const boost::function<void ()>& main) :
@@ -148,8 +152,14 @@ Thread::Thread(const boost::function<void ()>& main) :
 
 Thread::~Thread() {
     if (impl_ != NULL) {
-        // In case we didn't call wait yet
-        const int result = pthread_detach(impl_->tid_);
+        int result = pthread_detach(impl_->tid_);
+        // If the error indicates that thread doesn't exist but we're
+        // in child process (after fork) it is expected. We should
+        // not cause an assert.
+        if (result == ESRCH && !impl_->fork_detector_.isParent()) {
+            result = 0;
+        }
+
         Impl::done(impl_);
         impl_ = NULL;
         // If the detach ever fails, something is screwed rather badly.
@@ -164,13 +174,20 @@ Thread::wait() {
                   "Wait called and no thread to wait for");
     }
 
+    // Was there an exception in the thread?
+    scoped_ptr<UncaughtException> ex;
+
     const int result = pthread_join(impl_->tid_, NULL);
     if (result != 0) {
-        isc_throw(isc::InvalidOperation, std::strerror(result));
+        // We will not throw exception if the error indicates that the
+        // thread doesn't exist and we are in the child process (forked).
+        // For the child process it is expected that the thread is not
+        // re-created when we fork.
+        if (result != ESRCH || impl_->fork_detector_.isParent()) {
+            isc_throw(isc::InvalidOperation, std::strerror(result));
+        }
     }
 
-    // Was there an exception in the thread?
-    scoped_ptr<UncaughtException> ex;
     // Something here could in theory throw. But we already terminated the thread, so
     // we need to make sure we are in consistent state even in such situation (like
     // releasing the mutex and impl_).