Skip to content

[dotnet] [bidi] Simplified how background tasks are started#17198

Merged
nvborisenko merged 1 commit intoSeleniumHQ:trunkfrom
nvborisenko:bidi-simplify-task-start
Mar 10, 2026
Merged

[dotnet] [bidi] Simplified how background tasks are started#17198
nvborisenko merged 1 commit intoSeleniumHQ:trunkfrom
nvborisenko:bidi-simplify-task-start

Conversation

@nvborisenko
Copy link
Member

Remove TaskFactory to start tasks.

💥 What does this PR do?

This pull request refactors the way background tasks are started in the Broker and EventDispatcher classes. The changes simplify task creation by replacing the custom TaskFactory with the standard Task.Run method, making the code easier to read and maintain.

🔄 Types of changes

  • Cleanup (formatting, renaming)

Copilot AI review requested due to automatic review settings March 10, 2026 21:14
@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Mar 10, 2026
@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Simplify background task initialization using Task.Run

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replaced custom TaskFactory with standard Task.Run method
• Removed static TaskFactory instances from Broker and EventDispatcher
• Simplified background task initialization in two classes
• Improved code readability and maintainability

Grey Divider

File Changes

1. dotnet/src/webdriver/BiDi/Broker.cs ✨ Enhancement +1/-3

Replace TaskFactory with Task.Run in Broker

• Removed static _myTaskFactory field declaration
• Replaced _myTaskFactory.StartNew() with Task.Run() for receiving message task
• Simplified async task creation by removing TaskFactory wrapper

dotnet/src/webdriver/BiDi/Broker.cs


2. dotnet/src/webdriver/BiDi/EventDispatcher.cs ✨ Enhancement +1/-3

Replace TaskFactory with Task.Run in EventDispatcher

• Removed static _myTaskFactory field declaration
• Replaced _myTaskFactory.StartNew() with Task.Run() for event emitter task
• Simplified async task creation by removing TaskFactory wrapper

dotnet/src/webdriver/BiDi/EventDispatcher.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 10, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Broker Task.Run untested 📘 Rule violation ⛯ Reliability
Description
The background receive loop is now started via Task.Run, which changes scheduling/LongRunning
semantics and may affect reliability under load or during disposal. No corresponding test updates
are included to validate the new behavior.
Code

dotnet/src/webdriver/BiDi/Broker.cs[50]

+        _receivingMessageTask = Task.Run(() => ReceiveMessagesAsync(_receiveMessagesCancellationTokenSource.Token));
Evidence
PR Compliance ID 4 requires code changes to be accompanied by appropriate tests. The diff shows the
task-start mechanism was changed to Task.Run for the message receiving loop, but no tests are
shown/updated in the provided PR diff.

AGENTS.md
dotnet/src/webdriver/BiDi/Broker.cs[50-50]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Broker` now starts its receive loop with `Task.Run`, which can change runtime behavior (e.g., no `LongRunning` hint). The change should be validated with targeted unit tests to ensure the background task starts reliably and cancels/stops cleanly during disposal.

## Issue Context
The PR changes task startup behavior for background processing, which is a reliability-sensitive area.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Broker.cs[50-50]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors .NET BiDi background task startup by removing a custom TaskFactory and using Task.Run instead, simplifying task creation in the BiDi message broker and event dispatcher.

Changes:

  • Replace TaskFactory.StartNew(...).Unwrap() with Task.Run(...) in EventDispatcher.
  • Replace TaskFactory.StartNew(..., LongRunning).Unwrap() with Task.Run(...) in Broker.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
dotnet/src/webdriver/BiDi/EventDispatcher.cs Starts the event processing background loop using Task.Run instead of a static TaskFactory.
dotnet/src/webdriver/BiDi/Broker.cs Starts the message receiving loop using Task.Run, removing the custom TaskFactory usage.

@nvborisenko nvborisenko merged commit 6addb15 into SeleniumHQ:trunk Mar 10, 2026
22 of 25 checks passed
@nvborisenko nvborisenko deleted the bidi-simplify-task-start branch March 10, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants