Summary
Refactor ToolTask to use System.Threading.Channels for output data synchronization instead of lock/queue + ManualResetEvent pattern.
Background and Motivation
Currently, ToolTask uses a combination of Queue, ManualResetEvent, and locks to synchronize stdout and stderr output from child processes. This approach is complex and potentially error-prone. In PR #13351, a suggestion was made (and agreed to by @baronfel) to modernize this by using System.Threading.Channels, which would provide a cleaner and more robust producer-consumer abstraction.
The ManualResetEvent + Queue approach is especially challenging to reason about due to race conditions around signaling and resetting, and doesn't mesh well with modern async code. Channel would encapsulate most of the concurrency safety and make the code easier to maintain.
Proposed Feature
Refactor all relevant data flows in ToolTask (and any helper methods) that rely on enqueuing output lines and signaling availability to instead use Channel (or other appropriate channel type). Have the output-producing callbacks write directly to the channel, and update the consumer (HandleToolNotifications and downstream code) to pull from the channel (potentially asynchronously; see discussion below).
Preserve the priority ordering of tool notifications (timeout, cancel, error, output, exit) from the current WaitHandle.WaitAny pattern, but seek to simplify signaling and logistics by using channels where possible. If introducing async becomes problematic for the sync contract of ToolTask, investigate ways to integrate Channels with WaitHandle or use custom adapters.
Ensure all current regression tests around process output, timeouts, and hangs (e.g. #2981, #10378) are still passing. Consider breaking the refactor into multiple steps if necessary.
Alternative Designs
Summary
Refactor ToolTask to use System.Threading.Channels for output data synchronization instead of lock/queue + ManualResetEvent pattern.
Background and Motivation
Currently, ToolTask uses a combination of Queue, ManualResetEvent, and locks to synchronize stdout and stderr output from child processes. This approach is complex and potentially error-prone. In PR #13351, a suggestion was made (and agreed to by @baronfel) to modernize this by using System.Threading.Channels, which would provide a cleaner and more robust producer-consumer abstraction.
The ManualResetEvent + Queue approach is especially challenging to reason about due to race conditions around signaling and resetting, and doesn't mesh well with modern async code. Channel would encapsulate most of the concurrency safety and make the code easier to maintain.
Proposed Feature
Refactor all relevant data flows in ToolTask (and any helper methods) that rely on enqueuing output lines and signaling availability to instead use Channel (or other appropriate channel type). Have the output-producing callbacks write directly to the channel, and update the consumer (HandleToolNotifications and downstream code) to pull from the channel (potentially asynchronously; see discussion below).
Preserve the priority ordering of tool notifications (timeout, cancel, error, output, exit) from the current WaitHandle.WaitAny pattern, but seek to simplify signaling and logistics by using channels where possible. If introducing async becomes problematic for the sync contract of ToolTask, investigate ways to integrate Channels with WaitHandle or use custom adapters.
Ensure all current regression tests around process output, timeouts, and hangs (e.g. #2981, #10378) are still passing. Consider breaking the refactor into multiple steps if necessary.
Alternative Designs