Uh oh!
There was an error while loading. Please reload this page.
.NET: Remove timeout from InputWait in OffThread execution - #4996
Conversation
583e637 to
9db1bf8CompareThere was a problem hiding this comment.
Pull request overview
This PR updates the OffThread execution event-stream/run-loop coordination to remove the InputWait timeout (eliminating spurious “empty” steps) and fixes a race where an internal halt signal could be ignored if the consumer starts watching the event stream after the workflow already halted.
Changes:
- Remove the timeout from
StreamingRunEventStream’s wait-for-input behavior. - Fix
TakeEventStreamAsyncepoch selection so a pre-existing halt signal isn’t incorrectly ignored. - Add a unit test that delays stream consumption to validate the “halt before watch starts” scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI.Workflows/Execution/StreamingRunEventStream.cs | Removes input wait timeout and adjusts completion-epoch logic to avoid missing halt signals. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/InProcessExecutionTests.cs | Adds a regression test that starts watching the stream after execution has already been triggered. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…#4996) * fix: Remove Timeout from InputWait in StreamingRunEventStream * fix: Race condition when the workflow executes to halt before TakeEventStream * test: Make the OffThread Delay test more nimble * fix: Remove slight window where runStatus could be stale
Motivation and Context
When we first implemented OffThread running, we added a synchronization construct to avoid spinning a thread while the workflow is waiting on input after having run to Idle. Due to some race conditions we could not initially identify, we could end up with a deadlock, which we resolved by adding a timeout to the input wait. This could result in spurious "empty" steps, and it was long desired to remove it.
Over time we hardened the core of the runtime and removed the likely data races (particularly in the input rework for supporting Polymorphic Message Routing). We have high confidence (three thousand iterations of the unit tests, run locally) that these, along with the epoch fix in this PR, bring us to a good place regarding removing the timeout.
Description
Contribution Checklist
[ ] Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.