Skip to content

Conversation

TheButlah
Copy link
Collaborator

@TheButlah TheButlah commented Jun 26, 2025

Prior to this PR, code like the following would exhibit incorrect blocking-in-async behavior, if no agents were enabled.

impl Plan {
    async fn run(&mut self, orb: &mut Broker) -> Result<()> {
        let broker_fut = orb.run(self);
        let tout_fut = tokio::time::sleep(Duration::from_secs(4));
        tokio::select! {
            _ = tout_fut => info!("done sleeping"),
             // blocks forever the first time its polled - this prevents the timeout from ever triggering
            result = broker_fut => result?,
        }

        Ok(())
    }
}

This would happen because if no agents are enabled, the macro-generated poll() function for the future returned by Broker::run() would infinitely loop without ever returning. Now instead, it detects if no agents are enabled and returns Poll::pending, which ensures poll() 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:

impl Broker {
    fn poll_extra(
        &mut self,
        _plan: &mut dyn PlanT,
        _cx: &mut Context,
        _fence: std::time::Instant,
    ) -> Result<Option<Poll<()>>> {
        Ok(None)
    }
}

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.

@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch from 85b19f3 to 21e6d16 Compare June 26, 2025 17:07
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());
Copy link
Collaborator Author

@TheButlah TheButlah Jun 26, 2025

Choose a reason for hiding this comment

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

prior to this PR, the code would have blocked infinitely here.

@TheButlah TheButlah requested a review from valff June 26, 2025 17:10
@TheButlah TheButlah enabled auto-merge (squash) June 26, 2025 17:11
@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch 3 times, most recently from b16aa30 to 32f4ab5 Compare June 27, 2025 18:32
@TheButlah TheButlah added difficulty:nontrivial This PR is not trivial, and requires more detailed review tested:yes This PR was tested ai:none/search For PRs that make no use of AI (or only use it as a search engine) blocked:reviewer PR is waiting for the reviewer labels Jun 27, 2025
@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch 4 times, most recently from d277b59 to 1536046 Compare July 1, 2025 21:32
@TheButlah TheButlah disabled auto-merge July 1, 2025 21:32
@TheButlah TheButlah enabled auto-merge (squash) July 1, 2025 21:33
@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch 2 times, most recently from e9e0c89 to 4577f08 Compare July 10, 2025 13:49
@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch from 4577f08 to cb8984c Compare July 22, 2025 21:46
@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch from cb8984c to 5067742 Compare July 25, 2025 17:01
@TheButlah
Copy link
Collaborator Author

TheButlah commented Aug 8, 2025

@valff I would like to merge this pr soon, or at least get it reviewed.

@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch 2 times, most recently from ee2cbc8 to f350f93 Compare August 13, 2025 22:07
@TheButlah TheButlah force-pushed the thebutlah/agentwire-fix-infinite-loop branch from f350f93 to 76d60ca Compare August 26, 2025 19:30
@TheButlah TheButlah requested a review from a team as a code owner August 26, 2025 19:30
@sfikastheo sfikastheo force-pushed the thebutlah/agentwire-fix-infinite-loop branch from 76d60ca to 65f9ab0 Compare August 27, 2025 09:01
@AlexKaravaev
Copy link
Contributor

@valff could it be the reason for freezes of orb-core on diamond that we saw recently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ai:none/search For PRs that make no use of AI (or only use it as a search engine) blocked:reviewer PR is waiting for the reviewer difficulty:nontrivial This PR is not trivial, and requires more detailed review tested:yes This PR was tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants