RFC: run loop finish forward progress - #2217
Merged
ericniebler merged 2 commits intoAug 20, 2026
Merged
Conversation
|
Tested. Fix the issue. LGTM |
ericniebler
requested changes
Aug 20, 2026
ericniebler
left a comment
ericniebler
left a comment
Collaborator
There was a problem hiding this comment.
Nice, thanks. Can you pls clang-format the test file and add a CI job to run it under valgrind?
pointbazaar
force-pushed
the
run-loop-finish-forward-progress
branch
from
August 20, 2026 16:25
9389af0 to
0df3e7a
Compare
``` cmake -S . -B build -GNinja \ -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_CXX_STANDARD=20 \ -DCMAKE_CXX_EXTENSIONS=OFF \ -DSTDEXEC_BUILD_TESTS=ON cmake --build build --target test.run_loop_finish_repro -v ``` running it under valgrind easily results in 2+ minutes of runtime. The results are unreliable, sometimes it is faster, sometimes slower. valgrind --fair-sched=no ./build/test/test.run_loop_finish_repro but with fair scheduling it takes around 2 seconds reliably. valgrind --fair-sched=yes ./build/test/test.run_loop_finish_repro This indicates the implementation may depend on specific scheduling behavior which creates at least 60x difference in runtime. The issue i am trying to reproduce here is related to unit tests running under valgrind in other projects which rely on stdexec. Large unpredictable runtime differences on finishing the run loop can make their tests flaky since sometimes timeouts are exceeded. Next patch contains a fix. Co-Authored-By: GPT 5.6 Sol Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
The run loop drains queued work after finish() has been requested and also waits for __task_count_ to reach zero. If the queue becomes empty while the count is still non-zero, the current implementation busy-spins by repeatedly calling __execute_all(). This can starve another thread that still needs to make progress in order to decrement __task_count_. One concrete sequence is finish() incrementing the count by two, setting __finishing_, and queueing the noop task. The worker can then execute the noop, leaving the count at one, and immediately enter the tight drain loop. The caller still needs to execute its final fetch_sub() to bring the count to zero, but an unfair scheduler may keep running the spinning worker instead. This was reproducible under Valgrind's default scheduler as an intermittent hang during run_loop shutdown. Using Valgrind's fair scheduler avoided the hang, which pointed at a forward-progress issue rather than missing work. Yielding when there is no queued work but tasks are still in flight allows the thread responsible for completing those tasks to run. With this change, the reproducer completed 700 consecutive runs under Valgrind without --fair-sched=yes. Co-Authored-By: GPT 5.6 Sol Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
pointbazaar
force-pushed
the
run-loop-finish-forward-progress
branch
from
August 20, 2026 17:03
0df3e7a to
8897e22
Compare
Contributor
Author
|
@ericniebler i added a workflow step and separate build target to avoid running valgrind on all the existing tests in the CI matrix. i formatted the file and at least the formatting workflow ran through on my fork but the other workflows did not so not sure if it will work. Can you approve a trial run of the workflow? |
ericniebler
approved these changes
Aug 20, 2026
Collaborator
|
/ok to test 8897e22 |
Collaborator
|
looks like it worked. fantastic! thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running tests using
sdbusplus::async::context(which usesexecution::run_loop) i experienced occasional timeouts under valgrind's default unfair scheduler.This RFC PR contains a stdexec-only reproducer and fix attempt.
Looking for your feedback on my use-case and fix attempt 😃