[dotnet][bidi] Use channels dependency for producer/consumer

[nvborisenko]

User description

Motivation and Context

Now we are dependent on System.Threading.Channels package for netstandard2.0 target only. net8 is still free of dependencies.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Dependencies

---

Description

• Replaced BlockingCollection with Channel for event handling.

• Updated Broker class to use System.Threading.Channels.

• Added System.Threading.Channels as a dependency in multiple files.

• Improved task management and event processing loops.

---

Changes walkthrough 📝

	Relevant files
Enhancement	Broker.cs Refactored `Broker` to use `Channel` for events                    dotnet/src/webdriver/BiDi/Communication/Broker.cs Replaced BlockingCollection with Channel for event handling. Updated methods to use Channel for writing and reading events. Removed TaskFactory and improved task management. Renamed methods for better clarity and consistency. +14/-13
Dependencies	BUILD.bazel Added `System.Threading.Channels` to Bazel dependencies    dotnet/src/webdriver/BUILD.bazel Added System.Threading.Channels as a dependency for Bazel build. +2/-0      WebDriver.StrongNamed.nuspec Added `System.Threading.Channels` to StrongNamed nuspec    dotnet/src/webdriver/WebDriver.StrongNamed.nuspec Added System.Threading.Channels dependency for netstandard2.0. +1/-0      WebDriver.nuspec Added `System.Threading.Channels` to WebDriver nuspec        dotnet/src/webdriver/WebDriver.nuspec Added System.Threading.Channels dependency for netstandard2.0. +1/-0      WebDriver.csproj Added `System.Threading.Channels` to csproj dependencies  dotnet/src/webdriver/WebDriver.csproj Included System.Threading.Channels package for netstandard2.0. +1/-0      paket.dependencies Added `System.Threading.Channels` to Paket dependencies    dotnet/paket.dependencies Added System.Threading.Channels version 8.0.0 to Paket dependencies. +1/-0
Additional files	paket.nuget.bzl +10/-9

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The new event processing loop using channels should include proper error handling and graceful shutdown mechanisms when the channel is completed or errors occur private async Task ProcessEventsLoopAsync() { while (await _pendingEvents.Reader.WaitToReadAsync().ConfigureAwait(false)) { try { var result = await _pendingEvents.Reader.ReadAsync().ConfigureAwait(false); if (_eventHandlers.TryGetValue(result.Method, out var eventHandlers)) { if (eventHandlers is not null) Task Management The removal of TaskCreationOptions.LongRunning might impact performance for long-running operations. Verify that the new implementation handles long-running tasks efficiently _receivingMessageTask = Task.Run(async () => await ReceiveMessagesLoopAsync(_receiveMessagesCancellationTokenSource.Token)); _eventEmitterTask = Task.Run(ProcessEventsLoopAsync);

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add channel write error handling Add error handling for when writing to the channel fails. The WriteAsync operation could throw if the channel is completed or disposed. dotnet/src/webdriver/BiDi/Communication/Broker.cs [130] -await _pendingEvents.Writer.WriteAsync(messageEvent).ConfigureAwait(false); +try { + await _pendingEvents.Writer.WriteAsync(messageEvent).ConfigureAwait(false); +} catch (ChannelClosedException) { + // Handle or log channel closed error +} Apply this suggestion Suggestion importance[1-10]: 8 __ Why: Adding error handling for channel write operations is critical for preventing unhandled exceptions that could crash the application when the channel is completed or disposed.	Medium
General	Add cancellation support for channels Add cancellation token to channel operations to allow proper cancellation and cleanup during shutdown. dotnet/src/webdriver/BiDi/Communication/Broker.cs [142-146] -while (await _pendingEvents.Reader.WaitToReadAsync().ConfigureAwait(false)) +while (await _pendingEvents.Reader.WaitToReadAsync(_receiveMessagesCancellationTokenSource.Token).ConfigureAwait(false)) { try { - var result = await _pendingEvents.Reader.ReadAsync().ConfigureAwait(false); + var result = await _pendingEvents.Reader.ReadAsync(_receiveMessagesCancellationTokenSource.Token).ConfigureAwait(false); Apply this suggestion Suggestion importance[1-10]: 7 __ Why: Adding cancellation token support to channel operations is important for proper cleanup and resource management during shutdown, preventing potential resource leaks.	Medium
Learned best practice	Add null validation checks for method parameters to prevent potential NullReferenceExceptions The ExecuteCommandAsync<TCommand, TResult> and ExecuteCommandAsync methods accept a nullable options parameter but don't validate it before use. While it may be optional, explicitly validating nullable parameters helps prevent potential NullReferenceExceptions and makes the code more robust. dotnet/src/webdriver/BiDi/Communication/Broker.cs [182-185] public async Task<TResult> ExecuteCommandAsync<TCommand, TResult>(TCommand command, CommandOptions? options) where TCommand : Command { + ArgumentNullException.ThrowIfNull(command, nameof(command)); var jsonElement = await ExecuteCommandCoreAsync(command, options).ConfigureAwait(false); Apply this suggestion	Low

[RenderMichael]

Nice changes! How does this affect performance/memory?

[nvborisenko]

Nice changes! How does this affect performance/memory?

Didn't test it accurately without remote-end party. But I don't expect performance improvement here.

[nvborisenko]

Hm, now I am struggled: do we really want to introduce new dependency instead of BlockingCollection<> which just works? Given that a good library is a library without dependencies, then we should provide strong arguments why we need Channels.

[RenderMichael]

BlockingCollection<> is bounded to a maximum size, and is blocking (probably why we needed to mess with TaskFactory in the existing code).

Channel<T> is also much more low-level and is faster (performance improvements shown in the recent Nick Chapsas video and the older Toub blog post).

Additionally, this only adds a new dependency on older TFMs. I don't know the stats about how many people use the .NET 8 version vs. the .NET Standard version, but the kinds of blockers which prevent upgrades to modern .NET are not usually present in code that uses Selenium. For this reason, a dependency that only exists on older .NET versions seems less critical.

[nvborisenko]

Thanks @nvborisenko for contribution. I decided (8 Feb) to not introduce new dependency to fix mimic issue/improvement. In case of any issues, please came in here.