[dotnet] [bidi] Reuse memory when receiving websocket messages

[nvborisenko]

User description

🔗 Related Issues

💥 What does this PR do?

Improve memory allocation.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement, Bug fix

---

Description

• Reused a shared MemoryStream to optimize memory allocation.

• Updated ReceiveAsync to use the shared memory stream.

• Improved semaphore handling in SendAsync for better performance.

• Added proper disposal of shared resources in Dispose method.

---

Changes walkthrough 📝

	Relevant files
Enhancement	WebSocketTransport.cs Optimize memory and resource management in WebSocketTransport dotnet/src/webdriver/BiDi/Communication/Transport/WebSocketTransport.cs Introduced a shared MemoryStream for memory reuse. Modified ReceiveAsync to use the shared memory stream. Improved semaphore handling in SendAsync. Ensured proper disposal of shared resources in Dispose. +9/-6

---

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-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 1234 - Not compliant Non-compliant requirements: • Fix issue where Selenium 2.48 doesn't trigger JavaScript in link's href on click() • Ensure JavaScript in href attributes is properly executed when clicking links in Firefox 42.0 5678 - Not compliant Non-compliant requirements: • Fix "ConnectFailure (Connection refused)" errors when instantiating ChromeDriver multiple times • Address issues with ChromeDriver 2.35 on Ubuntu 16.04.4 with Chrome 65.0.3325.181

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Thread Safety The shared MemoryStream is not thread-safe. If multiple threads call ReceiveAsync concurrently, they could corrupt the shared memory stream's state. private readonly MemoryStream _sharedMemoryStream = new(); public async Task ConnectAsync(CancellationToken cancellationToken) { await _webSocket.ConnectAsync(_uri, cancellationToken).ConfigureAwait(false); } public async Task<byte[]> ReceiveAsync(CancellationToken cancellationToken) { _sharedMemoryStream.SetLength(0); WebSocketReceiveResult result; do { result = await _webSocket.ReceiveAsync(_receiveBuffer, cancellationToken).ConfigureAwait(false); await _sharedMemoryStream.WriteAsync(_receiveBuffer.Array!, _receiveBuffer.Offset, result.Count, cancellationToken).ConfigureAwait(false); } while (!result.EndOfMessage); byte[] data = _sharedMemoryStream.ToArray(); Semaphore Usage The semaphore acquisition pattern has changed. Verify that this doesn't introduce deadlocks or race conditions when cancellation occurs. var semaphoreTask = _socketSendSemaphoreSlim.WaitAsync(cancellationToken); try { await semaphoreTask.ConfigureAwait(false);

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Reset stream position The shared memory stream position isn't reset before calling ToArray(), which will return the entire buffer including any previously written data. Reset the position to 0 before calling ToArray(). dotnet/src/webdriver/BiDi/Communication/Transport/WebSocketTransport.cs [45-59] public async Task<byte[]> ReceiveAsync(CancellationToken cancellationToken) { _sharedMemoryStream.SetLength(0); WebSocketReceiveResult result; do { result = await _webSocket.ReceiveAsync(_receiveBuffer, cancellationToken).ConfigureAwait(false); await _sharedMemoryStream.WriteAsync(_receiveBuffer.Array!, _receiveBuffer.Offset, result.Count, cancellationToken).ConfigureAwait(false); } while (!result.EndOfMessage); + _sharedMemoryStream.Position = 0; byte[] data = _sharedMemoryStream.ToArray(); Apply this suggestion Suggestion importance[1-10]: 9 __ Why: This is a critical bug fix. Without resetting the stream position to 0 before calling ToArray(), the method would return the entire buffer including any previously written data, potentially causing data corruption. The PR removed the previous Seek(0, SeekOrigin.Begin) call when switching to a shared memory stream.	High
Learned best practice	Replace null-forgiving operators with proper null checks to prevent potential NullReferenceExceptions The code uses a null-forgiving operator (!) on _receiveBuffer.Array, which assumes the array is never null. Instead, add a proper null check or use null-conditional operators to safely handle potentially null values and prevent NullReferenceExceptions. dotnet/src/webdriver/BiDi/Communication/Transport/WebSocketTransport.cs [55] -await _sharedMemoryStream.WriteAsync(_receiveBuffer.Array!, _receiveBuffer.Offset, result.Count, cancellationToken).ConfigureAwait(false); +ArgumentNullException.ThrowIfNull(_receiveBuffer.Array, nameof(_receiveBuffer.Array)); +await _sharedMemoryStream.WriteAsync(_receiveBuffer.Array, _receiveBuffer.Offset, result.Count, cancellationToken).ConfigureAwait(false); Apply this suggestion Suggestion importance[1-10]: 6	Low
More