posix: Fix exec error reporting with limit_handles

_pipe_sink was assigned after call_on_setup(), after limit_fd_::on_setup(), but this was too late. It must be assigned earlier so that executor::get_used_handles() can see it and prevent limit_handles from closing the internal pipe for passing exec() errors from child to parent. Fixes: 1a1d677 Closes: #202 Signed-off-by: Daniel Klauer <[email protected]>
boostorg · Jun 4, 2024 · 0058a9c · 0058a9c
1 parent a26f4fe
commit 0058a9c
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 1 deletion.
diff --git a/include/boost/process/detail/posix/executor.hpp b/include/boost/process/detail/posix/executor.hpp
@@ -398,12 +398,16 @@ child executor<Sequence>::invoke(boost::mpl::false_, boost::mpl::false_)
             set_error(err, "fcntl(2) failed");//this might throw, so we need to be sure our pipe is safe.
             return child();
         }
+
+        _pipe_sink = p.p[1];
+
         _ec.clear();
         boost::fusion::for_each(seq, call_on_setup(*this));
 
         if (_ec)
         {
             boost::fusion::for_each(seq, call_on_error(*this, _ec));
+            _pipe_sink = -1;
             return child();
         }
 
@@ -417,11 +421,11 @@ child executor<Sequence>::invoke(boost::mpl::false_, boost::mpl::false_)
             _msg = "fork() failed";
             boost::fusion::for_each(seq, call_on_fork_error(*this, _ec));
             boost::fusion::for_each(seq, call_on_error(*this, _ec));
+            _pipe_sink = -1;
             return child();
         }
         else if (pid == 0)
         {
-            _pipe_sink = p.p[1];
             ::close(p.p[0]);
 
             boost::fusion::for_each(seq, call_on_exec_setup(*this));
@@ -439,6 +443,7 @@ child executor<Sequence>::invoke(boost::mpl::false_, boost::mpl::false_)
 
         ::close(p.p[1]);
         p.p[1] = -1;
+        _pipe_sink = -1;
         _read_error(p.p[0]);
 
     }

diff --git a/test/limit_fd.cpp b/test/limit_fd.cpp
@@ -193,3 +193,9 @@ BOOST_AUTO_TEST_CASE(limit_fd, *boost::unit_test::timeout(5))
 
     fclose(p);
 }
+
+BOOST_AUTO_TEST_CASE(limit_fd_does_not_break_error_reporting, *boost::unit_test::timeout(5))
+{
+    BOOST_CHECK_THROW(boost::process::system("/does/not/exist"), boost::process::process_error);
+    BOOST_CHECK_THROW(boost::process::system("/does/not/exist", boost::process::limit_handles), boost::process::process_error);
+}