[Boost-commit] svn:boost r49394 - in sandbox/SOC/2006/process/trunk: boost/process boost/process/detail libs/process/test

Subject: [Boost-commit] svn:boost r49394 - in sandbox/SOC/2006/process/trunk: boost/process boost/process/detail libs/process/test
From: jmmv84_at_[hidden]
Date: 2008-10-19 22:09:40

Author: jmmv
Date: 2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
New Revision: 49394
URL: http://svn.boost.org/trac/boost/changeset/49394

Log:
Deal with SIGCHLD when using Boost.Process instead of not managing it at all.
Fixes some weird problems where unit tests or programs under a debugger would
misteriously fail due to an aborted waitpid(2) (because the testing framework
and the debugger reprogram this signal instead of simply ignoring it).

Our approach: reset the signal to its default behavior.  Read the comments in
the detail/posix_sigchld.hpp file for more details on this.

Added:
   sandbox/SOC/2006/process/trunk/boost/process/detail/posix_sigchld.hpp   (contents, props changed)
   sandbox/SOC/2006/process/trunk/libs/process/test/detail_posix_sigchld_test.cpp   (contents, props changed)
   sandbox/SOC/2006/process/trunk/libs/process/test/include_detail_posix_sigchld_test.cpp   (contents, props changed)
Text files modified: 
   sandbox/SOC/2006/process/trunk/boost/process/child.hpp            |     2 ++                                      
   sandbox/SOC/2006/process/trunk/boost/process/detail/posix_ops.hpp |     3 +++                                     
   sandbox/SOC/2006/process/trunk/libs/process/test/Jamfile.v2       |     7 +++++++                                 
   3 files changed, 12 insertions(+), 0 deletions(-)

Modified: sandbox/SOC/2006/process/trunk/boost/process/child.hpp
==============================================================================
--- sandbox/SOC/2006/process/trunk/boost/process/child.hpp	(original)
+++ sandbox/SOC/2006/process/trunk/boost/process/child.hpp	2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
@@ -27,6 +27,7 @@
 #   include <sys/wait.h>
 }
 #   include <cerrno>
+#   include <boost/process/detail/posix_sigchld.hpp>
 #   include <boost/process/exceptions.hpp>
 #   include <boost/throw_exception.hpp>
 #elif defined(BOOST_PROCESS_WIN32_API)
@@ -228,6 +229,7 @@
         boost::throw_exception
             (system_error("boost::process::child::wait",
                           "waitpid(2) failed", errno));
+    detail::the_posix_sigchld_programmer.child_awaited();
     return status(s);
 #elif defined(BOOST_PROCESS_WIN32_API)
     DWORD code;

Modified: sandbox/SOC/2006/process/trunk/boost/process/detail/posix_ops.hpp
==============================================================================
--- sandbox/SOC/2006/process/trunk/boost/process/detail/posix_ops.hpp	(original)
+++ sandbox/SOC/2006/process/trunk/boost/process/detail/posix_ops.hpp	2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
@@ -41,6 +41,7 @@
 #include <boost/optional.hpp>
 #include <boost/process/detail/file_handle.hpp>
 #include <boost/process/detail/pipe.hpp>
+#include <boost/process/detail/posix_sigchld.hpp>
 #include <boost/process/detail/stream_info.hpp>
 #include <boost/process/environment.hpp>
 #include <boost/process/exceptions.hpp>
@@ -426,8 +427,10 @@
             info_map& infoout,
             const posix_setup& setup)
 {
+    the_posix_sigchld_programmer.forking_child();
     pid_t pid = ::fork();
     if (pid == -1) {
+        the_posix_sigchld_programmer.forking_failed();
         boost::throw_exception
             (system_error("boost::process::detail::posix_start",
                           "fork(2) failed", errno));

Added: sandbox/SOC/2006/process/trunk/boost/process/detail/posix_sigchld.hpp
==============================================================================
--- (empty file)
+++ sandbox/SOC/2006/process/trunk/boost/process/detail/posix_sigchld.hpp	2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
@@ -0,0 +1,194 @@
+//
+// Boost.Process
+//
+// Copyright (c) 2008 Julio M. Merino Vidal.
+//
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt or copy at
+// http://www.boost.org/LICENSE_1_0.txt.)
+//
+
+//!
+//! \file boost/process/detail/posix_sigchld.hpp
+//!
+//! A signal handler for SIGCHLD.
+//!
+
+#if !defined(BOOST_PROCESS_DETAIL_POSIX_SIGCHLD_HPP)
+/** \cond */
+#define BOOST_PROCESS_DETAIL_POSIX_SIGCHLD_HPP
+/** \endcond */
+
+#include <boost/process/config.hpp>
+
+#if !defined(BOOST_PROCESS_POSIX_API)
+#   error "Unsupported platform."
+#endif
+
+extern "C" {
+#   include <signal.h>
+}
+
+#   include <boost/assert.hpp>
+
+namespace boost {
+namespace process {
+namespace detail {
+
+// ------------------------------------------------------------------------
+
+//!
+//! \brief A model to reset SIGCHLD while using this library.
+//!
+//! The \a posix_sigchld_programmer class provides a model to handle the
+//! programming of the SIGCHLD signal handler while this library is
+//! managing children processes.
+//!
+//! The SIGCHLD signal must be handled in some way to prevent aborts of
+//! system calls when the handler is programmed (e.g. by Boost.Test or by
+//! a debugger).  There basically are two possibilities.  First, we reset
+//! the signal to its default behavior, which is to simply ignore it.  In
+//! this way, we do not get any system call premature aborts due to EINTR.
+//! The other approach is to capture the signal and do something useful
+//! with it, making all the blocking system calls we use safe to EINTR.
+//! We currently implement the former mechanism because we have no use for
+//! SIGCHLD yet.
+//!
+//! Note that handling EINTR but still not handling SIGCHLD in some way
+//! causes the tests to fail because Boost.Test installs a signal handler
+//! to monitor for unhandled signals.
+//!
+class posix_sigchld_programmer {
+    //!
+    //! \brief The amount of children processes the library is managing.
+    //!
+    int m_children_count;
+
+    //!
+    //! \brief The signal handler that was installed before reprogramming.
+    //!
+    struct sigaction m_old_handler;
+
+    //!
+    //! \brief Programs our own signal handler.
+    //!
+    //! This call is idempotent for simplicity of the callers.
+    //!
+    void program(void);
+
+    //!
+    //! \brief Restores the previous signal handler.
+    //!
+    //! This call is idempotent for simplicity of the callers.
+    //!
+    void unprogram(void);
+
+public:
+    posix_sigchld_programmer(void);
+    ~posix_sigchld_programmer(void);
+
+    //!
+    //! \brief Tells this model that we are about to spawn a child.
+    //!
+    void forking_child(void);
+
+    //!
+    //! \brief Tells this model that spawning a child failed, thus
+    //! canceling the actions of forking_child.
+    //!
+    void forking_failed(void);
+
+    //!
+    //! \brief Tells this model that a child's status has been collected
+    //! and is no further in zombie status.
+    //!
+    void child_awaited(void);
+};
+
+// ------------------------------------------------------------------------
+
+//!
+//! \brief The library-wide \a posix_sigchld_programmer instance.
+//!
+posix_sigchld_programmer the_posix_sigchld_programmer;
+
+// ------------------------------------------------------------------------
+
+inline
+posix_sigchld_programmer::posix_sigchld_programmer(void) :
+    m_children_count(0)
+{
+}
+
+// ------------------------------------------------------------------------
+
+inline
+posix_sigchld_programmer::~posix_sigchld_programmer(void)
+{
+}
+
+// ------------------------------------------------------------------------
+
+inline
+void
+posix_sigchld_programmer::program(void)
+{
+    if (m_children_count == 1) {
+        struct sigaction sa;
+        sa.sa_handler = SIG_DFL;
+        sa.sa_flags = 0;
+        sigemptyset(&sa.sa_mask);
+
+        ::sigaction(SIGCHLD, &sa, &m_old_handler);
+    }
+}
+
+// ------------------------------------------------------------------------
+
+inline
+void
+posix_sigchld_programmer::unprogram(void)
+{
+    if (m_children_count == 0)
+        ::sigaction(SIGCHLD, &m_old_handler, NULL);
+}
+
+// ------------------------------------------------------------------------
+
+inline
+void
+posix_sigchld_programmer::forking_child(void)
+{
+    m_children_count++;
+    program();
+}
+
+// ------------------------------------------------------------------------
+
+inline
+void
+posix_sigchld_programmer::forking_failed(void)
+{
+    BOOST_ASSERT(m_children_count > 0);
+    m_children_count--;
+    unprogram();
+}
+
+// ------------------------------------------------------------------------
+
+inline
+void
+posix_sigchld_programmer::child_awaited(void)
+{
+    BOOST_ASSERT(m_children_count > 0);
+    m_children_count--;
+    unprogram();
+}
+
+// ------------------------------------------------------------------------
+
+} // namespace detail
+} // namespace process
+} // namespace boost
+
+#endif // !defined(BOOST_PROCESS_DETAIL_POSIX_SIGCHLD_HPP)

Modified: sandbox/SOC/2006/process/trunk/libs/process/test/Jamfile.v2
==============================================================================
--- sandbox/SOC/2006/process/trunk/libs/process/test/Jamfile.v2	(original)
+++ sandbox/SOC/2006/process/trunk/libs/process/test/Jamfile.v2	2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
@@ -39,6 +39,7 @@
     [ compile include_detail_file_handle_test.cpp : : ]
     [ compile include_detail_pipe_test.cpp : : ]
     [ compile include_detail_posix_ops_test.cpp : : ]
+    [ compile include_detail_posix_sigchld_test.cpp : : ]
     [ compile include_detail_systembuf_test.cpp : : ]
     [ compile include_detail_win32_ops_test.cpp : : ]
     [ compile include_environment_test.cpp : : ]
@@ -97,6 +98,12 @@
         : <library>$(BOOST_ROOT)//unit_test_framework
         ]
 
+    [ run detail_posix_sigchld_test.cpp
+        :
+        :
+        : <library>$(BOOST_ROOT)//unit_test_framework
+        ]
+
     [ run stream_behavior_test.cpp
         :
         :

Added: sandbox/SOC/2006/process/trunk/libs/process/test/detail_posix_sigchld_test.cpp
==============================================================================
--- (empty file)
+++ sandbox/SOC/2006/process/trunk/libs/process/test/detail_posix_sigchld_test.cpp	2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
@@ -0,0 +1,141 @@
+//
+// Boost.Process
+// Regression tests for the detail::posix_sigchld class.
+//
+// Copyright (c) 2008 Julio M. Merino Vidal.
+//
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt or copy at
+// http://www.boost.org/LICENSE_1_0.txt.)
+//
+
+#include <boost/process/config.hpp>
+
+#if defined(BOOST_PROCESS_POSIX_API)
+#   include <sys/types.h>
+
+#   include <signal.h>
+#   include <unistd.h>
+
+#   include <boost/process/detail/posix_sigchld.hpp>
+
+namespace bpd = ::boost::process::detail;
+#endif
+
+#include <boost/test/unit_test.hpp>
+
+namespace but = ::boost::unit_test;
+
+// ------------------------------------------------------------------------
+
+#if defined(BOOST_PROCESS_POSIX_API)
+bool received = false;
+#endif
+
+// ------------------------------------------------------------------------
+
+#if defined(BOOST_PROCESS_POSIX_API)
+static
+void
+sigchld_handler(int signo)
+{
+    received = true;
+}
+#endif
+
+// ------------------------------------------------------------------------
+
+#if defined(BOOST_PROCESS_POSIX_API)
+static
+void
+install_sigchld_handler(void)
+{
+    received = false;
+
+    struct sigaction sa;
+    sa.sa_handler = sigchld_handler;
+    sa.sa_flags = 0;
+    sigemptyset(&sa.sa_mask);
+    BOOST_CHECK(::sigaction(SIGCHLD, &sa, NULL) != -1);
+}
+#endif
+
+// ------------------------------------------------------------------------
+
+#if defined(BOOST_PROCESS_POSIX_API)
+static
+void
+test_test_handler(void)
+{
+    install_sigchld_handler();
+    BOOST_CHECK(!received);
+    BOOST_CHECK(::kill(::getpid(), SIGCHLD) != -1);
+    BOOST_CHECK(received);
+}
+#endif
+
+// ------------------------------------------------------------------------
+
+#if defined(BOOST_PROCESS_POSIX_API)
+static
+void
+test_fork_fail(void)
+{
+    install_sigchld_handler();
+
+    bpd::the_posix_sigchld_programmer.forking_child();
+    BOOST_CHECK(::kill(::getpid(), SIGCHLD) != -1);
+    BOOST_CHECK(!received);
+
+    bpd::the_posix_sigchld_programmer.forking_failed();
+    BOOST_CHECK(::kill(::getpid(), SIGCHLD) != -1);
+    BOOST_CHECK(received);
+}
+#endif
+
+// ------------------------------------------------------------------------
+
+#if defined(BOOST_PROCESS_POSIX_API)
+static
+void
+test_fork_wait(void)
+{
+    install_sigchld_handler();
+
+    bpd::the_posix_sigchld_programmer.forking_child();
+    BOOST_CHECK(::kill(::getpid(), SIGCHLD) != -1);
+    BOOST_CHECK(!received);
+
+    bpd::the_posix_sigchld_programmer.child_awaited();
+    BOOST_CHECK(::kill(::getpid(), SIGCHLD) != -1);
+    BOOST_CHECK(received);
+}
+#endif
+
+// ------------------------------------------------------------------------
+
+#if !defined(BOOST_PROCESS_POSIX_API)
+static
+void
+test_dummy(void)
+{
+}
+#endif
+
+// ------------------------------------------------------------------------
+
+but::test_suite *
+init_unit_test_suite(int argc, char* argv[])
+{
+    but::test_suite* test = BOOST_TEST_SUITE("posix_child test suite");
+
+#if defined(BOOST_PROCESS_POSIX_API)
+    test->add(BOOST_TEST_CASE(&test_test_handler));
+    test->add(BOOST_TEST_CASE(&test_fork_fail));
+    test->add(BOOST_TEST_CASE(&test_fork_wait));
+#else
+    test->add(BOOST_TEST_CASE(&test_dummy));
+#endif
+
+    return test;
+}

Added: sandbox/SOC/2006/process/trunk/libs/process/test/include_detail_posix_sigchld_test.cpp
==============================================================================
--- (empty file)
+++ sandbox/SOC/2006/process/trunk/libs/process/test/include_detail_posix_sigchld_test.cpp	2008-10-19 22:09:39 EDT (Sun, 19 Oct 2008)
@@ -0,0 +1,30 @@
+//
+// Boost.Process
+// Checks that detail/posix_sigchld.hpp can be included without errors.
+//
+// Copyright (c) 2008 Julio M. Merino Vidal.
+//
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt or copy at
+// http://www.boost.org/LICENSE_1_0.txt.)
+//
+
+#include <boost/process/config.hpp>
+
+#if defined(BOOST_PROCESS_POSIX_API)
+
+#   include <boost/process/detail/posix_sigchld.hpp>
+
+namespace bpd = ::boost::process::detail;
+
+// ------------------------------------------------------------------------
+
+void*
+test_it(void)
+{
+    return &bpd::the_posix_sigchld_programmer;
+}
+
+// ------------------------------------------------------------------------
+
+#endif