From: Marcin Siodelski Date: Wed, 23 Sep 2015 11:58:54 +0000 (+0200) Subject: [3971] Resolved issues with fork/exec while spawning LFC. X-Git-Tag: trac4074_base~11^2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=973e98b726769100ec546c37d158e2aad24e43ca;p=thirdparty%2Fkea.git [3971] Resolved issues with fork/exec while spawning LFC. --- diff --git a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in index 6fe48bf7e0..5f8dab791d 100755 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/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}" diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index b124f87dd2..8263d9fbcd 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/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 diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am index 7db34c7326..b49db42bd5 100644 --- a/src/lib/util/Makefile.am +++ b/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 diff --git a/src/lib/util/fork_detector.h b/src/lib/util/fork_detector.h new file mode 100644 index 0000000000..aa30ee800d --- /dev/null +++ b/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 +#include + +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 diff --git a/src/lib/util/threads/thread.cc b/src/lib/util/threads/thread.cc index 8f0436b4ef..458bf83448 100644 --- a/src/lib/util/threads/thread.cc +++ b/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 #include #include @@ -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& main) : @@ -148,8 +152,14 @@ Thread::Thread(const boost::function& 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 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 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_).