Skip to content

Replace apr with boost in LLProcess#5988

Merged
akleshchev merged 44 commits into
developfrom
andreyk/boost_process_experiment
Jul 14, 2026
Merged

Replace apr with boost in LLProcess#5988
akleshchev merged 44 commits into
developfrom
andreyk/boost_process_experiment

Conversation

@akleshchev

@akleshchev akleshchev commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Replace apr's process logic with boost::process::v2

@akleshchev akleshchev marked this pull request as draft July 10, 2026 20:07
@akleshchev akleshchev force-pushed the andreyk/boost_process_experiment branch 5 times, most recently from 5db14a6 to e7aa1fd Compare July 10, 2026 20:39
@akleshchev akleshchev changed the base branch from release/26.3 to develop July 10, 2026 20:40
@akleshchev akleshchev force-pushed the andreyk/boost_process_experiment branch from e7aa1fd to e0d7b6d Compare July 10, 2026 20:48
@secondlife secondlife deleted a comment from github-actions Bot Jul 10, 2026
@akleshchev akleshchev force-pushed the andreyk/boost_process_experiment branch 5 times, most recently from 9008bcc to 0166827 Compare July 10, 2026 22:43
Copilot AI and others added 20 commits July 14, 2026 09:05
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Inherit LLProcess from std::enable_shared_from_this<LLProcess> and
capture a weak_ptr in the mainloop lambda. Locking the weak_ptr before
calling tick() keeps *this alive for the full callback duration, so a
listener that drops the last LLProcessPtr during a synchronous event
post inside tick()/handleExit() cannot destroy the object while it is
still on the call stack.
shared_from_this() requires the shared_ptr control block to be fully
initialized, which is not the case during the constructor. Move the
mainloop listener registration into a separate connectMainloop() method
and call it from create() after make_shared<LLProcess>() returns.
…code for bp::child

boost::process v1 only specializes initializer_tag<std::error_code> (in
error.hpp), not initializer_tag<boost::system::error_code>. On macOS with
Boost 1.90, these are distinct types, so passing boost::system::error_code
directly to bp::child() constructor caused a template instantiation failure.

Revert the two ec declarations in LLProcess::~LLProcess() and LLProcess::launch()
back to std::error_code, which is what boost::process v1 expects.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread indra/llcommon/llprocess.cpp
Comment thread indra/cmake/Boost.cmake
Comment thread indra/cmake/Boost.cmake
Comment thread indra/cmake/Boost.cmake
akleshchev and others added 6 commits July 14, 2026 14:46
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
boost::process v2, when constructed with an mIOContext, installs a
SIGCHLD handler via boost::asio without SA_RESTART. Without SA_RESTART,
blocking waitpid() calls elsewhere in the process return EINTR when a
child process exits, causing INTEGRATION_TEST_llsdserialize to fail with
'waitpid() errno 4: expected ECHILD(10) actual EINTR(4)'.

After launching the child process, explicitly read back the SIGCHLD
disposition and add SA_RESTART if missing, ensuring blocking syscalls
are automatically restarted after SIGCHLD delivery.

(The original boost::process v1 code explicitly warned about this exact
issue and worked around it by not passing mIOContext to bp::child; v2
requires the executor so we fix it post-launch instead.)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

indra/llcommon/llprocess.h:49

  • llprocess.h uses pid_t for non-Windows process IDs/handles, but after removing APR headers it no longer includes a header that defines pid_t. This makes the header fragile and can fail to compile depending on include order.
#if LL_WINDOWS
#include "llwin32headers.h" // for HANDLE
#elif LL_LINUX
#if defined(Status)
#undef Status

Comment thread indra/llcommon/llprocess.cpp
Comment thread indra/llcommon/llprocess.cpp Outdated
Comment thread indra/llcommon/llprocess.cpp Outdated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace apr in llprocess with boost::process

5 participants