fix(agentwire): fix infinite loop when no agents enabled#554
Open
TheButlah wants to merge 1 commit into
Open
Conversation
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
from
June 26, 2025 17:07
85b19f3 to
21e6d16
Compare
TheButlah
commented
Jun 26, 2025
| let mut no_agents = NoAgents::new(); | ||
| let mut run_fut = no_agents.run(&mut plan); | ||
| // poll should always immediately return, instead of looping forever. | ||
| assert!(run_fut.poll_unpin(&mut cx).is_pending()); |
Contributor
Author
There was a problem hiding this comment.
prior to this PR, the code would have blocked infinitely here.
TheButlah
enabled auto-merge (squash)
June 26, 2025 17:11
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
3 times, most recently
from
June 27, 2025 18:32
b16aa30 to
32f4ab5
Compare
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
4 times, most recently
from
July 1, 2025 21:32
d277b59 to
1536046
Compare
TheButlah
disabled auto-merge
July 1, 2025 21:32
TheButlah
enabled auto-merge (squash)
July 1, 2025 21:33
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
2 times, most recently
from
July 10, 2025 13:49
e9e0c89 to
4577f08
Compare
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
from
July 22, 2025 21:46
4577f08 to
cb8984c
Compare
tonalove23
approved these changes
Jul 22, 2025
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
from
July 25, 2025 17:01
cb8984c to
5067742
Compare
Contributor
Author
|
@valff I would like to merge this pr soon, or at least get it reviewed. |
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
2 times, most recently
from
August 13, 2025 22:07
ee2cbc8 to
f350f93
Compare
TheButlah
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
from
August 26, 2025 19:30
f350f93 to
76d60ca
Compare
sfikastheo
force-pushed
the
thebutlah/agentwire-fix-infinite-loop
branch
from
August 27, 2025 09:01
76d60ca to
65f9ab0
Compare
Contributor
|
@valff could it be the reason for freezes of orb-core on diamond that we saw recently? |
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
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.
Prior to this PR, code like the following would exhibit incorrect blocking-in-async behavior, if no agents were enabled.
This would happen because if no agents are enabled, the macro-generated
poll()function for the future returned byBroker::run()would infinitely loop without ever returning. Now instead, it detects if no agents are enabled and returns Poll::pending, which ensurespoll()is always non-blocking.I've fixed the offending code and added a test case for it.
NOTE: This PR is only a partial fix. There is still an unhandled edge case, where a user defined poll_extra can cause the same infinite-blocking behavior:
However, this is not a regression - this also happens prior to this PR. Fixing this would be desirable, but even the partial fix in this PR is useful - therefore I would like to land this PR first and deal with that additional edge case later.