From 014f99a53a3ab733c0992dcd4c14a90e63ed55b3 Mon Sep 17 00:00:00 2001 From: romanett Date: Sun, 18 Oct 2026 14:37:56 +0200 Subject: [PATCH] WebSocket transport: a sender queued across Close fails as closed (#4832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `WebSocketByteTransportTests.CloseFailsQueuedSendInsteadOfOrphaningItAsync` fails intermittently. One example is test-linux-net10.0 (3/16) in run 39114639155 on #4831, an unrelated MachineTool change. The queued send failed with a raw `WebSocketException` ("…transitioned into an invalid state") instead of `ServiceResultException` with `BadConnectionClosed`. ## Cause This is a product race in `WebSocketByteTransportBase`. - `Close()` cancels the shared send token. That wakes both the in-flight send and the queued `SemaphoreSlim.WaitAsync`. - `SemaphoreSlim` removes a cancelled waiter on a later continuation. If the in-flight sender's `finally` releases the lock first, the release *grants* the queued waiter despite its cancelled token. - `EnterSendAsync` then returned normally, and the queued sender called `SendAsync` on the already aborted socket. - That socket error is not an `OperationCanceledException`, so it reached the caller untranslated. ## Fix After acquiring the lock, `EnterSendAsync` checks whether the transport closed. If it did, it gives the lock back and throws `BadConnectionClosed`. `Close()` sets the closed flag before it cancels, so every waiter woken by `Close()` sees it. ## Tests - The existing test body moved into a helper. New `CloseFailsQueuedSendEvenWhenTheLockIsGrantedAsync` repeats the interleaving 2000 times (about 140 ms). - Without the fix it fails 3/3 runs within about 100 ms, with the CI exception. With the fix it passes 3/3. - `Opc.Ua.Core.Tests` `~WebSocket` (net10.0): 39/39 passed. đŸ¤– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 --- .../Https/WebSocketByteTransport.cs | 11 ++++++++++ .../Transport/WebSocketByteTransportTests.cs | 22 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Opc.Ua.Bindings.Https/Https/WebSocketByteTransport.cs b/src/Opc.Ua.Bindings.Https/Https/WebSocketByteTransport.cs index 408de881dd..94e46aab84 100644 --- a/src/Opc.Ua.Bindings.Https/Https/WebSocketByteTransport.cs +++ b/src/Opc.Ua.Bindings.Https/Https/WebSocketByteTransport.cs @@ -393,6 +393,17 @@ namespace Opc.Ua.Bindings try { await m_sendLock.WaitAsync(linkedCts.Token).ConfigureAwait(false); + // SemaphoreSlim can still grant a waiter whose token was cancelled when a + // release reaches it before the cancellation removes it. A sender queued + // across Close would then write to the aborted socket and fail with the + // socket's own error, so it gives the lock back and fails as closed. + if (Volatile.Read(ref m_closed) != 0) + { + m_sendLock.Release(); + throw ServiceResultException.Create( + StatusCodes.BadConnectionClosed, + "The WebSocket transport is closed."); + } return linkedCts; } catch (OperationCanceledException) when (!ct.IsCancellationRequested) diff --git a/tests/Opc.Ua.Core.Tests/Stack/Transport/WebSocketByteTransportTests.cs b/tests/Opc.Ua.Core.Tests/Stack/Transport/WebSocketByteTransportTests.cs index f25139b708..f91123a46a 100644 --- a/tests/Opc.Ua.Core.Tests/Stack/Transport/WebSocketByteTransportTests.cs +++ b/tests/Opc.Ua.Core.Tests/Stack/Transport/WebSocketByteTransportTests.cs @@ -503,7 +503,27 @@ namespace Opc.Ua.Core.Tests.Stack.Transport /// sender forever. /// [Test] - public async Task CloseFailsQueuedSendInsteadOfOrphaningItAsync() + public Task CloseFailsQueuedSendInsteadOfOrphaningItAsync() + { + return CloseWithQueuedSendAsync(); + } + + /// + /// Close can wake the in-flight sender, whose lock release then grants + /// the queued waiter before its cancellation removes it. The queued + /// sender must still fail as closed rather than write to the aborted + /// socket. The interleaving depends on the thread pool, so it repeats. + /// + [Test] + public async Task CloseFailsQueuedSendEvenWhenTheLockIsGrantedAsync() + { + for (int iteration = 0; iteration < 2000; iteration++) + { + await CloseWithQueuedSendAsync().ConfigureAwait(false); + } + } + + private async Task CloseWithQueuedSendAsync() { using var socket = new BlockingSendWebSocket(); var transport = new WebSocketServerByteTransport(