diff --git a/src/lib/util/threads/tests/thread_unittest.cc b/src/lib/util/threads/tests/thread_unittest.cc index a334c7576f..82afbcb2b2 100644 --- a/src/lib/util/threads/tests/thread_unittest.cc +++ b/src/lib/util/threads/tests/thread_unittest.cc @@ -14,14 +14,18 @@ #include +#include +#include #include #include #include +#include #include #include +#include // This file tests the Thread class. It's hard to test an actual thread is // started, but we at least check the function is run and exceptions are @@ -34,38 +38,161 @@ // started in parallel (the other tests wait for the previous one to terminate // before starting new one). +using namespace isc::util; using namespace isc::util::thread; namespace { const size_t iterations = 200; const size_t detached_iterations = 25; -void -doSomething(int*) { } +/// @brief Implements a thread which can be stopped. +/// +/// This class implements a worker thread which can be stopped by calling +/// StoppableThread::stop. The call to this function blocks until the thread +/// terminates. This class is useful for testing scenarios when the thread +/// needs to be run for a specific amount of time. +class StoppableThread { +public: + + /// @brief Constructor. + /// + /// It doesn't run the thread yet. It merely initializes required + /// class members. + StoppableThread() + : stopping_(false), mutex_(), thread_() { + } + + /// @brief Destructor. + /// + /// Detaches the thread. + ~StoppableThread() { + } + + /// @brief Starts the execution of the thread. + /// + /// This method will not start the thread if the thread has been stopped. + /// In order to start the thread again, @c StoppableThread::reset must be + /// called first. + void run() { + if (!amStopping()) { + thread_.reset(new Thread(boost::bind(&StoppableThread::loop, this))); + } + } + + /// @brief Stops the thread as soon as possible. + void stop() { + if (!amStopping()) { + setStopping(true); + if (thread_) { + thread_->wait(); + thread_.reset(); + } + } + } + + /// @brief Resets the thread state so as it can be ran again. + void reset() { + setStopping(false); + } + +private: + + /// @brief Checks if the thread is being stopped. + /// + /// @return true if the thread is being stopped. + bool amStopping() { + Mutex::Locker lock(mutex_); + return (stopping_); + } + + /// @brief Sets the stopping state. + /// + /// @param stopping New value for @c stopping_ state. + void setStopping(const bool stopping) { + Mutex::Locker lock(mutex_); + stopping_ = stopping; + } + + /// @brief Worker thread loop. + /// + /// It runs until the @c StoppableThread::stop is called. + void loop() { + for (;;) { + if (amStopping()) { + break; + } + usleep(100); + } + } + + /// @brief Flag indicating if the thread is being stopped. + bool stopping_; + /// @brief Mutex used for protecting @c stopping_ flag. + Mutex mutex_; + /// @brief Pointer to the thread instance. + boost::scoped_ptr thread_; +}; + +/// @brief Static instance of the stoppable thread. +boost::scoped_ptr thread; + +class ThreadTest : public ::testing::Test { +public: + + virtual ~ThreadTest() { + if (thread) { + thread->stop(); + } + thread.reset(); + } + + /// @brief No-op method. + static void doSomething(int*) { } + + /// @brief Marks specified boolean value as true to indicate that the + /// function has been run. + static void markRun(bool* mark) { + EXPECT_FALSE(*mark); + *mark = true; + } + + /// @brief Throws 42. + static void throwSomething() { + throw 42; // Throw something really unusual, to see everything is caught. + } + + /// @brief Throws standard exception. + static void throwException() { + throw std::exception(); + } + + /// @brief Returns signal mask set for a thread. + /// + /// @parm mask Pointer to signal mask set for the calling thread. + static void getSignalMask(sigset_t* mask) { + pthread_sigmask(SIG_SETMASK, 0, mask); + } +}; + + // We just test that we can forget about the thread and nothing // bad will happen on our side. -TEST(ThreadTest, detached) { +TEST_F(ThreadTest, detached) { if (!isc::util::unittests::runningOnValgrind()) { int x; for (size_t i = 0; i < detached_iterations; ++i) { - Thread thread(boost::bind(&doSomething, &x)); + Thread thread(boost::bind(&ThreadTest::doSomething, &x)); } } } -void -markRun(bool* mark) { - EXPECT_FALSE(*mark); - *mark = true; -} - // Wait for a thread to end first. The variable must be set at the time. -TEST(ThreadTest, wait) { +TEST_F(ThreadTest, wait) { if (!isc::util::unittests::runningOnValgrind()) { for (size_t i = 0; i < iterations; ++i) { bool mark = false; - Thread thread(boost::bind(markRun, &mark)); + Thread thread(boost::bind(&ThreadTest::markRun, &mark)); thread.wait(); ASSERT_TRUE(mark) << "Not finished yet in " << i << "th iteration"; // Can't wait second time @@ -74,30 +201,20 @@ TEST(ThreadTest, wait) { } } -void -throwSomething() { - throw 42; // Throw something really unusual, to see everything is caught. -} - -void -throwException() { - throw std::exception(); -} - // Exception in the thread we forget about should not do anything to us -TEST(ThreadTest, detachedException) { +TEST_F(ThreadTest, detachedException) { if (!isc::util::unittests::runningOnValgrind()) { for (size_t i = 0; i < detached_iterations; ++i) { - Thread thread(throwSomething); + Thread thread(&ThreadTest::throwSomething); } for (size_t i = 0; i < detached_iterations; ++i) { - Thread thread(throwException); + Thread thread(&ThreadTest::throwException); } } } // An uncaught exception in the thread should propagate through wait -TEST(ThreadTest, exception) { +TEST_F(ThreadTest, exception) { if (!isc::util::unittests::runningOnValgrind()) { for (size_t i = 0; i < iterations; ++i) { Thread thread(throwSomething); @@ -108,21 +225,57 @@ TEST(ThreadTest, exception) { } } -// Returns signal mask set for a thread. -void -getSignalMask(sigset_t* mask) { - pthread_sigmask(SIG_SETMASK, 0, mask); -} - // Verify that all signals are blocked. -TEST(ThreadTest, sigmask) { +TEST_F(ThreadTest, sigmask) { sigset_t mask; sigemptyset(&mask); - Thread thread(boost::bind(getSignalMask, &mask)); + Thread thread(boost::bind(&ThreadTest::getSignalMask, &mask)); ASSERT_NO_THROW(thread.wait()); EXPECT_EQ(1, sigismember(&mask, SIGHUP)); EXPECT_EQ(1, sigismember(&mask, SIGINT)); EXPECT_EQ(1, sigismember(&mask, SIGTERM)); } + +/// The @c ProcessSpawn class spawns new processes using the fork/exec +/// scheme. If the call to exec fails, child process exits with the +/// EXIT_FAILURE status code. The call to exit triggers destruction of +/// all static objects, i.e. destructors of statically declared +/// @c Thread objects are called in the child process. The call to +/// fork doesn't clone threads existing in the main process. So, the +/// @c Thread objects in the child process have invalid state, because +/// they point to non-existing threads. As a result any attempts to +/// detach or join the thread may result in errors or asserts. +/// +/// This test verifies that the @c Thread class protects against such +/// errors. It is supposed to detect that the @c Thread object is in +/// the child process and not assert when the destruction fails. +TEST_F(ThreadTest, spawnProcessWithThread) { + // Initialize and run the stoppable thread. Note that the 'thread' + // is a static variable, which will be 'cloned' into the child + // process. Its destructor will be called when the child process + // terminates with EXIT_FAILURE status. So in fact the destructor + // of the @c StoppableThread will be called twice: in the main + // process and in the child process. + thread.reset(new StoppableThread()); + thread->run(); + + // Spawn the new process, using some non-existing executable. The + // current process will fork but the execvp should fail. + ProcessSpawn process_spawn("kea-dhcp4-a86570943h"); + pid_t pid = process_spawn.spawn(); + // Wait for the process to terminate. + while (process_spawn.isRunning(pid)) { + usleep(100); + } + // When the child process terminates it will call destructors of + // static objects. This means that it will call the destructor of + // the 'thread' object too. The 'thread' object in the child + // process points to non-existing thread, but we expect the + // graceful destruction, i.e. the destructor should not assert. + // If the destructor asserts the exit code returned here will + // be 0 - same as if we aborted. + EXPECT_EQ(EXIT_FAILURE, process_spawn.getExitStatus(pid)); +} + } diff --git a/src/lib/util/threads/thread.cc b/src/lib/util/threads/thread.cc index 458bf83448..7da5f99cbb 100644 --- a/src/lib/util/threads/thread.cc +++ b/src/lib/util/threads/thread.cc @@ -152,6 +152,7 @@ Thread::Thread(const boost::function& main) : Thread::~Thread() { if (impl_ != NULL) { + 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