[dotnet] [bidi] Rent and return buffer to shared pool per remote message

[nvborisenko]

User description

🔗 Related Issues

Contributes to #16801 and continuation of #16804

💥 What does this PR do?

Instead of renting a memory at the beginning of BiDi lifecycle, why not to rent it exactly when we need it?

🔧 Implementation Notes

It removes complexity of finalizers.

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Enhancement

---

Description

• Rent buffer from pool only when needed in ReceiveAsync

• Return buffer immediately after use via try-finally

• Remove finalizer and buffer field from class

• Simplify disposal logic by eliminating buffer cleanup

---

Diagram Walkthrough

flowchart LR
  A["ReceiveAsync called"] -->|"Rent buffer"| B["Use buffer in try block"]
  B -->|"Return buffer in finally"| C["Buffer released to pool"]
  D["Remove _receiveBuffer field"] -->|"Simplify"| E["Remove finalizer"]
  E -->|"Simplify"| F["Remove Dispose cleanup"]

Loading

File Walkthrough

Relevant files

Enhancement

WebSocketTransport.cs Aggressive buffer pooling with finalizer removal                  dotnet/src/webdriver/BiDi/WebSocketTransport.cs Removed _receiveBuffer field and rents buffer locally in ReceiveAsync method Wrapped buffer usage in try-finally block to ensure return to shared pool Removed finalizer (~WebSocketTransport) to eliminate complexity Simplified Dispose method by removing buffer cleanup logic Fixed segment write to use receiveBuffer directly instead of segment properties +23/-28

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #16801 🟢 🔴 ⚪ Fix the regression introduced in Selenium.WebDriver 4.39.0 that causes intermittent System.Text.Json parsing failures (including when taking screenshots) in .NET environments, especially on CI/Linux. Ensure JSON responses/data handled by Selenium are not corrupted and remain parseable under real-world loads and concurrent test execution. Provide a change that restores reliability comparable to 4.38.0 across affected environments (e.g., GitHub Actions on Ubuntu and Windows Server).
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs raw payload: The new/modified trace logging records the full received BiDi payload which may include sensitive data depending on message contents. Referred Code if (_logger.IsEnabled(LogEventLevel.Trace)) { _logger.Trace($"BiDi RCV <-- {Encoding.UTF8.GetString(data)}"); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use a local MemoryStream for thread safety To ensure thread safety, replace the shared _sharedMemoryStream field with a local MemoryStream instance inside the ReceiveAsync method. dotnet/src/webdriver/BiDi/WebSocketTransport.cs [46-76] var receiveBuffer = ArrayPool<byte>.Shared.Rent(1024 * 8); - try { - _sharedMemoryStream.SetLength(0); + using var localStream = new MemoryStream(1024 * 8); ArraySegment<byte> segment = new(receiveBuffer); WebSocketReceiveResult result; do { result = await _webSocket.ReceiveAsync(segment, cancellationToken).ConfigureAwait(false); - _sharedMemoryStream.Write(receiveBuffer, 0, result.Count); + localStream.Write(receiveBuffer, 0, result.Count); } while (!result.EndOfMessage); - byte[] data = _sharedMemoryStream.ToArray(); + byte[] data = localStream.ToArray(); ... return data; } finally { ArrayPool<byte>.Shared.Return(receiveBuffer); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential race condition by using a shared, non-thread-safe _sharedMemoryStream. Replacing it with a local MemoryStream instance within ReceiveAsync is a robust solution to prevent data corruption from concurrent calls.	Medium
Possible issue	Add WebSocket close‐frame handling Add logic to handle WebSocket close frames by checking result.MessageType and initiating a clean close handshake. dotnet/src/webdriver/BiDi/WebSocketTransport.cs [56-62] do { result = await _webSocket.ReceiveAsync(segment, cancellationToken).ConfigureAwait(false); + if (result.MessageType == WebSocketMessageType.Close) + { + await _webSocket.CloseAsync(WebSocketCloseStatus.NormalClosure, "Closing", cancellationToken).ConfigureAwait(false); + return Array.Empty<byte>(); + } _sharedMemoryStream.Write(receiveBuffer, 0, result.Count); } while (!result.EndOfMessage); Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly points out missing handling for WebSocket close frames. Implementing this logic is crucial for robustly managing the WebSocket lifecycle and preventing the application from misinterpreting a close message as data.	Medium
More