Merge pull request #227 from x-santiaga-x/patch-1

Closes klemens-morgenstern/boost-process#218

include/boost/process/detail/child_decl.hpp

@@ -60,7 +60,7 @@ public:
     explicit child(child_handle &&ch, const std::shared_ptr<std::atomic<int>> &ptr) : _child_handle(std::move(ch)), _exit_status(ptr) {}
     explicit child(child_handle &&ch) : _child_handle(std::move(ch)) {}
 
-    explicit child(const pid_t & pid) : _child_handle(pid), _attached(false) {};
+    explicit child(pid_t pid) : _child_handle(pid), _attached(false) {};
     child(const child&) = delete;
     child(child && lhs) noexcept
         : _child_handle(std::move(lhs._child_handle)),

include/boost/process/detail/posix/async_pipe.hpp

@@ -25,6 +25,11 @@ public:
     typedef ::boost::asio::posix::stream_descriptor handle_type;
     typedef typename handle_type::executor_type executor_type;
 
+    executor_type get_executor()
+    {
+        return _source.get_executor();
+    }
+
     inline async_pipe(boost::asio::io_context & ios) : async_pipe(ios, ios) {}
 
     inline async_pipe(boost::asio::io_context & ios_source,
@@ -45,8 +50,8 @@ public:
     inline async_pipe(const async_pipe& lhs);
     async_pipe(async_pipe&& lhs)  : _source(std::move(lhs._source)), _sink(std::move(lhs._sink))
     {
-        lhs._source.assign (-1);
-        lhs._sink  .assign (-1);
+        lhs._source = ::boost::asio::posix::stream_descriptor{lhs._source.get_executor()};
+        lhs._sink   = ::boost::asio::posix::stream_descriptor{lhs._sink.  get_executor()};
     }
 
     template<class CharT, class Traits = std::char_traits<CharT>>

include/boost/process/detail/posix/environment.hpp

@@ -56,6 +56,7 @@ public:
     {
         _buffer = _load();
         _impl = _load_var(_buffer);
+        _env_impl = _impl.data();
     }
 
     string_type get(const pointer_type id) { return get(string_type(id)); }

include/boost/process/detail/posix/executor.hpp

@@ -152,11 +152,10 @@ class executor
     void write_error(const std::error_code & ec, const char * msg)
     {
         //I am the child
-        int len = ec.value();
-        ::write(_pipe_sink, &len, sizeof(int));
+        const auto len = std::strlen(msg);
+        int data[2] = {ec.value(), len + 1};
 
-        len = std::strlen(msg) + 1;
-        ::write(_pipe_sink, &len, sizeof(int));
+        ::write(_pipe_sink, &data[0], sizeof(int) * 2);
         ::write(_pipe_sink, msg, len);
     }
 
@@ -273,15 +272,15 @@ class executor
         prepare_cmd_style_fn = exe;
         if ((prepare_cmd_style_fn.find('/') == std::string::npos) && ::access(prepare_cmd_style_fn.c_str(), X_OK))
         {
-            auto e = ::environ;
+            const auto * e = ::environ;
             while ((e != nullptr) && (*e != nullptr) && !boost::starts_with(*e, "PATH="))
                 e++;
 
             if ((e != nullptr) && (*e != nullptr))
             {
-                *e += 5; //the beginnig of the string contains "PATH="
                 std::vector<std::string> path;
-                boost::split(path, *e, boost::is_any_of(":"));
+                //the beginning of the string contains "PATH="
+                boost::split(path, (*e) + 5, boost::is_any_of(":"));
 
                 for (const std::string & pp : path)
                 {

include/boost/process/detail/posix/on_exit.hpp

@@ -6,26 +6,39 @@
 #ifndef BOOST_PROCESS_POSIX_ON_EXIT_HPP_
 #define BOOST_PROCESS_POSIX_ON_EXIT_HPP_
 
+#include <boost/asio/execution.hpp>
+#include <boost/process/async.hpp>
 #include <boost/process/detail/config.hpp>
 #include <boost/process/detail/handler_base.hpp>
 #include <boost/process/detail/posix/async_handler.hpp>
 #include <system_error>
 #include <functional>
 
-namespace boost { namespace process { namespace detail { namespace posix {
+namespace boost { namespace process { namespace detail {
+
+template<typename Tuple>
+inline asio::io_context& get_io_context(const Tuple & tup);
+
+namespace posix {
 
 struct on_exit_ : boost::process::detail::posix::async_handler
 {
     std::function<void(int, const std::error_code&)> handler;
     on_exit_(const std::function<void(int, const std::error_code&)> & handler) : handler(handler)
     {
-
     }
 
     template<typename Executor>
-    std::function<void(int, const std::error_code&)> on_exit_handler(Executor&) 
+    std::function<void(int, const std::error_code&)> on_exit_handler(Executor& exec)
     {
-        return handler;
+        auto v = boost::asio::prefer(boost::process::detail::get_io_context(exec.seq).get_executor(),
+                                     boost::asio::execution::outstanding_work.tracked);
+        auto handler_ = this->handler;
+        return
+            [handler_, v](int exit_code, const std::error_code & ec)
+            {
+                handler_(exit_code, ec);
+            };
 
     }
 };

include/boost/process/detail/posix/sigchld_service.hpp

@@ -48,9 +48,22 @@ public:
                     int status;
                     auto pid_res = ::waitpid(pid, &status, WNOHANG);
                     if (pid_res < 0)
-                        h(-1, get_last_error());
+                    {
+                        auto ec = get_last_error();
+                        boost::asio::post(
+                                _strand,
+                                [pid_res, ec, h]
+                                {
+                                    h(pid_res, ec);
+                                });
+                    }
                     else if ((pid_res == pid) && (WIFEXITED(status) || WIFSIGNALED(status)))
-                        h(status, {}); //successfully exited already
+                        boost::asio::post(
+                                _strand,
+                                [status, h]
+                                {
+                                    h(status, {}); //successfully exited already
+                                });
                     else //still running
                     {
                         if (_receivers.empty())

include/boost/process/detail/posix/terminate.hpp

@@ -27,7 +27,7 @@ inline void terminate(const child_handle &p, std::error_code &ec) noexcept
         ec.clear();
 
     int status;
-    ::waitpid(p.pid, &status, WNOHANG); //just to clean it up
+    ::waitpid(p.pid, &status, 0); //should not be WNOHANG, since that would allow zombies.
 }
 
 inline void terminate(const child_handle &p)

include/boost/process/detail/posix/wait_for_exit.hpp

@@ -158,7 +158,7 @@ inline bool wait_until(
         {
             int res;
             ::kill(pid, SIGKILL);
-            ::waitpid(pid, &res, WNOHANG);
+            ::waitpid(pid, &res, 0);
         }
     };
     child_cleaner_t child_cleaner{timeout_pid};

include/boost/process/detail/traits/wchar_t.hpp

@@ -7,13 +7,13 @@
 #ifndef BOOST_PROCESS_DETAIL_TRAITS_WCHAR_T_HPP_
 #define BOOST_PROCESS_DETAIL_TRAITS_WCHAR_T_HPP_
 
+#include <algorithm>
+
 #include <boost/process/detail/traits/decl.hpp>
 #include <boost/process/detail/traits/cmd_or_exe.hpp>
 #include <boost/process/detail/traits/env.hpp>
 #include <boost/process/locale.hpp>
 
-#include <algorithm>
-
 namespace boost { namespace process { namespace detail {
 
 //template

include/boost/process/detail/windows/io_context_ref.hpp

@@ -8,7 +8,9 @@
 
 #include <boost/process/detail/handler_base.hpp>
 #include <boost/process/detail/windows/async_handler.hpp>
+#include <boost/process/detail/windows/is_running.hpp>
 #include <boost/asio/io_context.hpp>
+#include <boost/asio/post.hpp>
 #include <boost/asio/windows/object_handle.hpp>
 #include <boost/winapi/process.hpp>
 #include <boost/winapi/handles.hpp>
@@ -114,6 +116,15 @@ struct io_context_ref : boost::process::detail::handler_base
 
         wait_handler wh(std::move(funcs), ios, process_handle, exec.exit_status);
 
+        ::boost::winapi::DWORD_ code;
+        if(::boost::winapi::GetExitCodeProcess(process_handle, &code)
+            && code != still_active)
+        {
+            ::boost::asio::post(wh.handle->get_executor(), std::move(wh));
+            return;
+        }
+
+
         auto handle_p = wh.handle.get();
         handle_p->async_wait(std::move(wh));
     }
@@ -130,12 +141,13 @@ struct io_context_ref : boost::process::detail::handler_base
                      boost::asio::io_context & ios, void * handle,
                      const std::shared_ptr<std::atomic<int>> &exit_status)
                 : funcs(std::move(funcs)),
-                  handle(new boost::asio::windows::object_handle(ios.get_executor(), handle)),
+                  handle(new boost::asio::windows::object_handle(
+                          asio::prefer(ios.get_executor(), asio::execution::outstanding_work.tracked), handle)),
                   exit_status(exit_status)
         {
 
         }
-        void operator()(const boost::system::error_code & ec_in)
+        void operator()(const boost::system::error_code & ec_in = {})
         {
             std::error_code ec;
             if (ec_in)

include/boost/process/detail/windows/is_running.hpp

@@ -7,6 +7,7 @@
 #define BOOST_PROCESS_WINDOWS_IS_RUNNING_HPP
 
 #include <boost/process/detail/config.hpp>
+#include <boost/process/detail/windows/child_handle.hpp>
 #include <system_error>
 #include <cstdlib>
 #include <boost/winapi/process.hpp>

include/boost/process/detail/windows/on_exit.hpp

@@ -6,13 +6,20 @@
 #ifndef BOOST_PROCESS_WINDOWS_ON_EXIT_HPP_
 #define BOOST_PROCESS_WINDOWS_ON_EXIT_HPP_
 
+#include <boost/process/async.hpp>
 #include <boost/process/detail/config.hpp>
 #include <boost/process/detail/handler_base.hpp>
 #include <boost/process/detail/windows/async_handler.hpp>
+#include <boost/asio/execution.hpp>
 #include <system_error>
 #include <functional>
 
-namespace boost { namespace process { namespace detail { namespace windows {
+namespace boost { namespace process { namespace detail {
+
+template<typename Tuple>
+inline asio::io_context& get_io_context(const Tuple & tup);
+
+namespace windows {
 
 struct on_exit_ : boost::process::detail::windows::async_handler
 {
@@ -23,10 +30,12 @@ struct on_exit_ : boost::process::detail::windows::async_handler
     }
 
     template<typename Executor>
-    std::function<void(int, const std::error_code&)> on_exit_handler(Executor&)
+    std::function<void(int, const std::error_code&)> on_exit_handler(Executor& exec)
     {
+        auto v = boost::asio::prefer(boost::process::detail::get_io_context(exec.seq).get_executor(),
+                                     boost::asio::execution::outstanding_work.tracked);
         auto handler_ = this->handler;
-        return [handler_](int exit_code, const std::error_code & ec)
+        return [v, handler_](int exit_code, const std::error_code & ec)
                {
                     handler_(static_cast<int>(exit_code), ec);
                };

include/boost/process/pipe.hpp

@@ -279,7 +279,7 @@ private:
         else if (wrt == 0) //broken pipe
             return false;
 
-        this->pbump(-wrt);
+        this->pbump(static_cast<int>(-wrt));
 
         return true;
     }

test/appveyor.yml

@@ -13,6 +13,7 @@
 init:
     - set BRANCH_TO_TEST=%APPVEYOR_REPO_BRANCH%
     - set BOOST_REMOVE=process
+    - set BOOST_TEST_LOG_LEVEL=success
 
 os: Visual Studio 2015
 configuration: Debug

test/async.cpp

@@ -182,8 +182,8 @@ BOOST_AUTO_TEST_CASE(async_wait_different_contexts, *boost::unit_test::timeout(1
     BOOST_REQUIRE(!ec);
 
     // Regression test for #143: make sure each io_context handles its own children
-    std::thread thr1{[&]{io_context1.run();}};
-    std::thread thr2{[&]{io_context2.run();}};
+    std::thread thr1{[&]() noexcept {io_context1.run();}};
+    std::thread thr2{[&]() noexcept {io_context2.run();}};
 
     thr1.join();
     thr2.join();
@@ -214,7 +214,7 @@ BOOST_AUTO_TEST_CASE(async_wait_abort, *boost::unit_test::timeout(5))
     int exit_code = 0;
     bp::child c(
         master_test_suite().argv[1],
-        "test", "--abort",
+        "test", "exit-code", "42",
         ec,
         io_context,
         bp::on_exit([&](int exit, const std::error_code& ec_in)
@@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(async_wait_abort, *boost::unit_test::timeout(5))
     io_context.run();
 
     BOOST_CHECK(exit_called);
-    BOOST_CHECK_NE(exit_code,  0);
+    BOOST_CHECK_NE(exit_code,  42);
     BOOST_CHECK_EQUAL(c.exit_code(), exit_code);
 }
 
@@ -413,4 +413,5 @@ BOOST_AUTO_TEST_CASE(mixed_async, *boost::unit_test::timeout(5))
 
 }*/
 
+
 BOOST_AUTO_TEST_SUITE_END();

test/limit_fd.cpp

@@ -167,14 +167,14 @@ BOOST_AUTO_TEST_CASE(limit_fd, *boost::unit_test::timeout(5))
     const auto get_handle = [](FILE * f){return std::to_string(fileno(f));};
 #endif
 
+    auto p = fopen("./test-file", "w");
+
     using boost::unit_test::framework::master_test_suite;
-    
-    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle",  get_handle(stdin), bp::std_err > stderr), EXIT_SUCCESS);
-    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle",  get_handle(stderr), bp::std_err > stderr), EXIT_SUCCESS);
-
-
-    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle", get_handle(stdin), bp::std_err > stderr, bp::limit_handles), EXIT_FAILURE);
-    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle", get_handle(stderr), bp::std_err > stderr, bp::limit_handles), EXIT_SUCCESS);
 
+    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle", bp::limit_handles, get_handle(p),  bp::std_in  < p), EXIT_SUCCESS);
+    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle", bp::limit_handles, get_handle(p),  bp::std_err > p), EXIT_SUCCESS);
+    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle", bp::limit_handles, get_handle(p),  bp::std_out > p), EXIT_SUCCESS);
+    BOOST_CHECK_EQUAL(bp::system(master_test_suite().argv[1], "--has-handle", bp::limit_handles, get_handle(p)), EXIT_FAILURE);
 
+    fclose(p);
 }