pid_test fixes for osx.

Closes #175

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

@@ -155,13 +155,15 @@ private:
 
 std::vector<char*> exe_cmd_init<char>::make_cmd()
 {
+    // any string must be writable.
+    static char empty_string[1] = "";
     std::vector<char*> vec;
     if (!exe.empty())
-        vec.push_back(&exe.front());
+        vec.push_back(exe.empty() ? empty_string : &exe.front());
 
     if (!args.empty()) {
         for (auto & v : args)
-            vec.push_back(&v.front());
+            vec.push_back(v.empty() ? empty_string : &v.front());
     }
 
     vec.push_back(nullptr);

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

@@ -7,8 +7,11 @@
 #ifndef BOOST_PROCESS_DETAIL_POSIX_SIGCHLD_SERVICE_HPP_
 #define BOOST_PROCESS_DETAIL_POSIX_SIGCHLD_SERVICE_HPP_
 
+#include <boost/asio/bind_executor.hpp>
 #include <boost/asio/dispatch.hpp>
 #include <boost/asio/post.hpp>
+#include <boost/asio/consign.hpp>
+#include <boost/asio/append.hpp>
 #include <boost/asio/signal_set.hpp>
 #include <boost/asio/strand.hpp>
 #include <boost/optional.hpp>
@@ -26,6 +29,43 @@ class sigchld_service : public boost::asio::detail::service_base<sigchld_service
 
     std::list<std::pair<::pid_t, std::function<void(int, std::error_code)>>> _receivers;
     inline void _handle_signal(const boost::system::error_code & ec);
+
+    struct initiate_async_wait_op
+    {
+        sigchld_service * self;
+        template<typename Initiation>
+        void operator()(Initiation && init, ::pid_t pid)
+        {
+            // check if the child actually is running first
+            int status;
+            auto pid_res = ::waitpid(pid, &status, WNOHANG);
+            if (pid_res < 0)
+            {
+                auto ec = get_last_error();
+                boost::asio::post(
+                        self->_strand,
+                        asio::append(std::forward<Initiation>(init), pid_res, ec));
+            }
+            else if ((pid_res == pid) && (WIFEXITED(status) || WIFSIGNALED(status)))
+                boost::asio::post(
+                        self->_strand,
+                        boost::asio::append(std::forward<Initiation>(init), status, std::error_code{}));
+            else //still running
+            {
+                sigchld_service * self_ = self;
+                if (self->_receivers.empty())
+                    self->_signal_set.async_wait(
+                        boost::asio::bind_executor(
+                            self->_strand,
+                            [self_](const boost::system::error_code &ec, int)
+                            {
+                                self_->_handle_signal(ec);
+                            }));
+                self->_receivers.emplace_back(pid, init);
+            }
+        }
+    };
+
 public:
     sigchld_service(boost::asio::io_context & io_context)
         : boost::asio::detail::service_base<sigchld_service>(io_context)
@@ -37,47 +77,10 @@ public:
         void (int, std::error_code))
     async_wait(::pid_t pid, SignalHandler && handler)
     {
-        boost::asio::async_completion<
-        SignalHandler, void(boost::system::error_code)> init{handler};
-
-        auto & h = init.completion_handler;
-        boost::asio::dispatch(
-                _strand,
-                [this, pid, h]
-                {
-                    //check if the child actually is running first
-                    int status;
-                    auto pid_res = ::waitpid(pid, &status, WNOHANG);
-                    if (pid_res < 0)
-                    {
-                        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)))
-                        boost::asio::post(
-                                _strand,
-                                [status, h]
-                                {
-                                    h(status, {}); //successfully exited already
-                                });
-                    else //still running
-                    {
-                        if (_receivers.empty())
-                            _signal_set.async_wait(
-                                    [this](const boost::system::error_code &ec, int)
-                                    {
-                                        boost::asio::dispatch(_strand, [this, ec]{this->_handle_signal(ec);});
-                                    });
-                        _receivers.emplace_back(pid, h);
-                    }
-                });
-
-        return init.result.get();
+        return boost::asio::async_initiate<
+            SignalHandler,
+            void(int, std::error_code)>(
+                initiate_async_wait_op{this}, handler, pid);
     }
     void shutdown() override
     {

include/boost/process/v2/environment.hpp

@@ -488,7 +488,7 @@ struct key
     using string_type      = std::basic_string<char_type, traits_type>;
     using string_view_type = basic_string_view<char_type, traits_type>;
 
-    key() noexcept = default;
+    key() {}
     key( const key& p ) = default;
     key( key&& p ) noexcept = default;
     key( const string_type& source ) : value_(source) {}
@@ -524,7 +524,11 @@ struct key
     ~key() = default;
 
     key& operator=( const key& p ) = default;
-    key& operator=( key&& p ) noexcept(std::is_nothrow_move_constructible<string_type>::value) = default;
+    key& operator=( key&& p )
+    {
+      value_ = std::move(p.value_);
+      return *this;
+    }
     key& operator=( string_type&& source )
     {
         value_ = std::move(source);
@@ -708,7 +712,7 @@ struct value
     using string_type      = std::basic_string<char_type, traits_type>;
     using string_view_type = basic_cstring_ref<char_type, traits_type>;
 
-    value() noexcept = default;
+    value() {}
     value( const value& p ) = default;
 
     value( const string_type& source ) : value_(source) {}
@@ -742,7 +746,11 @@ struct value
     ~value() = default;
 
     value& operator=( const value& p ) = default;
-    value& operator=( value&& p ) noexcept(std::is_nothrow_move_constructible<string_type>::value) = default;
+    value& operator=( value&& p )
+    {
+      value_ = std::move(p.value_);
+      return *this;
+    }
     value& operator=( string_type&& source )
     {
         value_ = std::move(source);
@@ -935,7 +943,7 @@ struct key_value_pair
     using string_type      = std::basic_string<char_type>;
     using string_view_type = basic_cstring_ref<char_type>;
 
-    key_value_pair() noexcept = default;
+    key_value_pair() {}
     key_value_pair( const key_value_pair& p ) = default;
     key_value_pair( key_value_pair&& p ) noexcept = default;
     key_value_pair(key_view key, value_view value) : value_(key.basic_string<char_type, traits_type>() + equality_sign + 
@@ -999,7 +1007,11 @@ struct key_value_pair
     ~key_value_pair() = default;
 
     key_value_pair& operator=( const key_value_pair& p ) = default;
-    key_value_pair& operator=( key_value_pair&& p ) noexcept(std::is_nothrow_move_constructible<string_type>::value) = default;
+    key_value_pair& operator=( key_value_pair&& p )
+    {
+      value_ = std::move(p.value_);
+      return *this;
+    }
     key_value_pair& operator=( string_type&& source )
     {
         value_ = std::move(source);

include/boost/process/v2/ext/impl/cmd.ipp

@@ -92,13 +92,12 @@ struct make_cmd_shell_
             str_lengths += (std::strlen(*c) + 1);
         }
         // yes, not the greatest solution.
-        std::string buffer;
         res.buffer_.resize(str_lengths);
 
         res.argv_ = new char*[res.argc_ + 1];
         res.free_argv_ = +[](int argc, char ** argv) {delete[] argv;};
         res.argv_[res.argc_] = nullptr;
-        auto p = &buffer[sizeof(int) * (res.argc_) + 1];
+        auto p = &*res.buffer_.begin();
 
         for (int i = 0; i < res.argc_; i++)
         {
@@ -305,9 +304,8 @@ shell cmd(boost::process::v2::pid_type pid, boost::system::error_code & ec)
         BOOST_PROCESS_V2_ASSIGN_LAST_ERROR(ec)
         return {};
     }
-    
     auto res = make_cmd_shell_::clone(cmd);
-    procstat_freeargv(proc_stat);
+    procstat_freeargv(proc_stat.get());
     return res;
 }
     

include/boost/process/v2/ext/impl/env.ipp

@@ -342,7 +342,7 @@ env_view env(boost::process::v2::pid_type pid, boost::system::error_code & ec)
         {
           auto eno = reinterpret_cast<char**>(out);
           auto eeo = eno;
-          auto str = out +  (n * sizeof(char*)) + sizeof(char*);
+            auto str = out +  (n * sizeof(char*)) + sizeof(char*);
           e = env;
           while (*e != nullptr)
           {
@@ -351,11 +351,11 @@ env_view env(boost::process::v2::pid_type pid, boost::system::error_code & ec)
             *eno = str;
             str += len;
             eno ++;
+            e++;
           }
           *eno = nullptr;
 
           ev.handle_.reset(eeo);
-
         }
         else
             BOOST_PROCESS_V2_ASSIGN_LAST_ERROR(ec)

include/boost/process/v2/impl/pid.ipp

@@ -147,15 +147,14 @@ std::vector<pid_type> child_pids(pid_type pid, boost::system::error_code & ec)
 std::vector<pid_type> all_pids(boost::system::error_code & ec)
 {
     std::vector<pid_type> vec;
-    vec.reserve(proc_listpids(PROC_ALL_PIDS, 0, nullptr, 0));
-    if (proc_listpids(PROC_ALL_PIDS, 0, &vec[0], sizeof(pid_type) * vec.size()))
+    vec.resize(proc_listpids(PROC_ALL_PIDS, 0, nullptr, 0) / sizeof(pid_type));
+    const auto sz = proc_listpids(PROC_ALL_PIDS, 0, &vec[0], sizeof(pid_type) * vec.size());
+    if (sz < 0)
     {
         BOOST_PROCESS_V2_ASSIGN_LAST_ERROR(ec)
         return {};
     }
-    auto itr = std::partition(vec.begin(), vec.end(), [](pid_type pt) {return pt != 0;});
-    vec.erase(itr, vec.end());
-    std::reverse(vec.begin(), vec.end());
+    vec.resize(sz);
     return vec;
 }
 
@@ -176,15 +175,14 @@ pid_type parent_pid(pid_type pid, boost::system::error_code & ec)
 std::vector<pid_type> child_pids(pid_type pid, boost::system::error_code & ec) 
 {
     std::vector<pid_type> vec;
-    vec.reserve(proc_listpids(PROC_PPID_ONLY, (uint32_t)pid, nullptr, 0));
-    if (proc_listpids(PROC_PPID_ONLY, (uint32_t)pid, &vec[0], sizeof(pid_type) * vec.size()))
+    vec.resize(proc_listpids(PROC_PPID_ONLY, (uint32_t)pid, nullptr, 0) / sizeof(pid_type));
+    const auto sz = proc_listpids(PROC_PPID_ONLY, (uint32_t)pid, &vec[0], sizeof(pid_type) * vec.size());
+    if (sz < 0)
     {
         BOOST_PROCESS_V2_ASSIGN_LAST_ERROR(ec)
         return {};
     }
-    auto itr = std::partition(vec.begin(), vec.end(), [](pid_type pt) {return pt != 0;});
-    vec.erase(itr, vec.end());
-    std::reverse(vec.begin(), vec.end());
+    vec.resize(sz);
     return vec;
 }
 

include/boost/process/v2/impl/shell.ipp

@@ -115,7 +115,7 @@ void shell::parse_()
 
 shell::~shell()
 {
-    if (argv_ != nullptr && free_argv_)
+    if (argv_ != nullptr && free_argv_ != nullptr)
         free_argv_(argc_, argv_);
 }
 

test/async_pipe.cpp

@@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(multithreaded_async_pipe)
     asio::io_context ioc;
 
     std::vector<std::thread> threads;
-    for (int i = 0; i < std::thread::hardware_concurrency(); i++)
+    for (auto i = 0u; i < std::thread::hardware_concurrency(); i++)
     {
         threads.emplace_back([&ioc]
         {

test/cmd_test.cpp

@@ -83,3 +83,24 @@ BOOST_AUTO_TEST_CASE(implicit)
         BOOST_TEST_MESSAGE(ec.message());
     BOOST_CHECK_EQUAL(ret, 21);
 }
+
+BOOST_AUTO_TEST_CASE(empty_cmd)
+{
+    using boost::unit_test::framework::master_test_suite;
+
+    std::error_code ec;
+
+    fs::path pth = master_test_suite().argv[1];
+    auto env = boost::this_process::environment();
+
+    auto itr = std::find_if(env.begin(), env.end(),
+    [](const bp::native_environment::entry_type & e){return boost::to_upper_copy(e.get_name()) == "PATH";});
+
+    BOOST_REQUIRE(itr != env.end());
+
+    (*itr) += fs::canonical(fs::absolute(pth.parent_path())).string();
+    BOOST_REQUIRE(itr != env.end());
+
+    bp::system("sparring_partner \"\" ", ec);
+}
+

test/group_wait.cpp

@@ -78,7 +78,7 @@ BOOST_AUTO_TEST_CASE(wait_group_test, *boost::unit_test::timeout(5))
     BOOST_CHECK_MESSAGE(!ec, ec.message());
     BOOST_REQUIRE(c2.in_group(ec));
     BOOST_CHECK_MESSAGE(!ec, ec.message());
-    g.wait();
+    g.wait(ec);
 
     BOOST_CHECK(!c1.running());
     BOOST_CHECK(!c2.running());

test/v2/Jamfile.jam

@@ -59,11 +59,11 @@ test-suite standalone :
     [ run utf8.cpp test_impl ]
     [ run cstring_ref.cpp test_impl ]
     [ run environment.cpp test_impl ]
-    [ run pid.cpp   test_impl : : : <target-os>darwin:<build>no ]
     [ run shell.cpp test_impl ]
     ;
 
 test-suite with_target :
+    [ run pid.cpp     test_impl : --log_level=all --catch_system_errors=no -- : target ]
     [ run process.cpp test_impl : --log_level=all --catch_system_errors=no -- : target ]
     [ run windows.cpp test_impl : --log_level=all --catch_system_errors=no -- : target : <build>no <target-os>windows:<build>yes <target-os>windows:<source>Advapi32 ]
     [ run ext.cpp     test_impl : --log_level=all --catch_system_errors=no -- : target : <target-os>darwin:<build>no ]

test/v2/ext.cpp

@@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(cmd_exe)
     BOOST_CHECK_EQUAL(bp2::detail::conv_string<char>(ref.data(), ref.size()), pth);
 
     BOOST_REQUIRE_EQUAL(cm.argc(), args.size() + 1);
-    for (auto i = 0; i < args.size(); i++)
+    for (auto i = 0u; i < args.size(); i++)
     {
       ref = cm.argv()[i + 1];
 

test/v2/pid.cpp

@@ -5,10 +5,12 @@
 // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
 
 #include <boost/process/v2/pid.hpp>
+#include <boost/process/v2/process.hpp>
 
 #include <boost/test/unit_test.hpp>
 
 #include <algorithm>
+#include <thread>
 #include <vector>
 
 BOOST_AUTO_TEST_CASE(test_pid)
@@ -22,21 +24,30 @@ BOOST_AUTO_TEST_CASE(test_pid)
     BOOST_CHECK_GT(all.size(), 0u);
     BOOST_CHECK(itr != all.end());
 
-    std::vector<bp2::pid_type> children, grand_children; 
-    auto grand_child_pids = [](bp2::pid_type pid, 
-        std::vector<bp2::pid_type> & children, 
-        std::vector<bp2::pid_type> & grand_children) 
-    {  
-        children = bp2::child_pids(pid);
-        for (unsigned i = 0; i < children.size(); i++) 
-        {
-            std::vector<bp2::pid_type> tmp1;
-            std::vector<bp2::pid_type> tmp2 = bp2::child_pids(children[i]);
-            tmp1.insert(std::end(tmp1), std::begin(tmp2), std::end(tmp2));
-            grand_children = tmp1;
-        }
-        return (!children.empty() || !grand_children.empty());
-    };
-    BOOST_CHECK_NE(grand_child_pids(bp2::root_pid, children, grand_children), false);
+
 
 }
+
+BOOST_AUTO_TEST_CASE(child_pid)
+{
+    namespace bp2 = boost::process::v2;
+
+    using boost::unit_test::framework::master_test_suite;
+    const auto pth = bp2::filesystem::absolute(master_test_suite().argv[1]);
+    std::this_thread::sleep_for(std::chrono::milliseconds(20));
+
+    auto cs = bp2::child_pids(bp2::current_pid());
+    boost::asio::io_context ctx;
+    bp2::process proc(ctx, pth, {"sleep", "50000"});
+    std::this_thread::sleep_for(std::chrono::milliseconds(20));
+    auto c2 = bp2::child_pids(bp2::current_pid());
+    BOOST_CHECK_LT(cs.size(), c2.size());
+    BOOST_CHECK(std::find(cs.begin(), cs.end(), proc.id()) == cs.end());
+    BOOST_CHECK(std::find(c2.begin(), c2.end(), proc.id()) != c2.end());
+    proc.terminate();
+    proc.wait();
+
+    auto c3 = bp2::child_pids(bp2::current_pid());
+    BOOST_CHECK(std::find(c3.begin(), c3.end(), proc.id()) == c3.end());
+    BOOST_CHECK_LT(c3.size(), c2.size());
+}