Skip to content

Restore original 7-field ReadPipe event contract in Boost.Asio implementation#5988

Draft
akleshchev wants to merge 29 commits into
developfrom
andreyk/boost_process_experiment
Draft

Restore original 7-field ReadPipe event contract in Boost.Asio implementation#5988
akleshchev wants to merge 29 commits into
developfrom
andreyk/boost_process_experiment

Conversation

@akleshchev

@akleshchev akleshchev commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

The new Boost.Asio-based ReadPipeImpl broke the original APR implementation's event contract in two ways: it posted only 4–5 fields per event instead of 7, and at EOF it split the final data and the eof signal into two separate events rather than one combined event.

Original APR contract — every pump post included all 7 fields:

data, len, slot, name, desc, eof, exhst

What was broken:

  • name (e.g. "stdout", "stderr") was never included
  • eof boolean was absent from normal data events
  • exhst boolean was absent from all events
  • data was omitted when mLimit == 0 (original always emitted it, as an empty string)
  • At EOF with pending bytes: two events were posted (data event + separate eof event) instead of one combined event

Fix in llprocess.cpp: Both the normal-read and EOF branches now always post all 7 fields. The async read states map to the original APR state machine as follows:

Async event eof exhst
Successful read (was EXHAUSTED) false true
EOF (was CLOSED) true false

Fix in llprocess_test.cpp: Reverted the setLimit() test (test 20) workaround — history.back() is correct again since the last event is the combined eof+data event.

Related Issues

  • Please link to a relevant GitHub issue for additional context.
    • Bug Fix: Link to an issue that includes reproduction steps and testing guidance.
    • Feature/Enhancement: Link to an issue with a write-up, rationale, and requirements.

Issue Link:


Checklist

Please ensure the following before requesting review:

  • I have provided a clear title and detailed description for this pull request.
  • If useful, I have included media such as screenshots and video to show off my changes.
  • The PR is linked to a relevant issue with sufficient context.
  • I have tested the changes locally and verified they work as intended.
  • All new and existing tests pass.
  • Code follows the project's style guidelines.
  • Documentation has been updated if needed.
  • Any dependent changes have been merged and published in downstream modules
  • I have reviewed the contributing guidelines.

Additional Notes

The exhst field semantics in the async model: Boost.Asio's async_read_some returns whenever any bytes are available, which corresponds to EXHAUSTED (read until no more data, pipe still open). There is no RETRY loop equivalent in the async model, so exhst is always true on non-EOF events and false on the EOF event.

@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 changed the title Fix LLProcess large-write scheduling that stalls LLLeap Restore original ReadPipe EOF event contract Jul 12, 2026
Copilot AI changed the title Restore original ReadPipe EOF event contract Restore original 7-field ReadPipe event contract in Boost.Asio implementation Jul 12, 2026
@akleshchev akleshchev force-pushed the andreyk/boost_process_experiment branch from 920d5f7 to 5e1418b Compare July 12, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants