See https://github.com/aspnet/Home/issues/2462
We see customers having a frequent issue with WebSockets, where they do something like this:
app.Use(async (context, next) => {
var socket = await context.WebSockets.AcceptWebSocketAsync();
BackgroundSocketProcessor.AddSocket(socket);
});
BackgroundSocketProcessor is some service or component in their app that adds the socket to a collection. A background thread, or other process (other requests, etc) are then accessing those sockets and sending data. This is a common pattern and pretty much how SignalR works, so it's not entirely wrong :). However, there's a problem with this as written. After adding the socket to the background processor queue, the middleware function returns. This completes the request and causes Kestrel to conclude the Response and close the stream. As a result, when the background processor tries to send/receive on the socket, it throws an ObjectDisposedException. The message is obscure (though it will be clarified soon: https://github.com/aspnet/KestrelHttpServer/issues/2299 :tada: and https://github.com/aspnet/Home/issues/2854) and difficult to understand. We should include documentation in our WebSockets topic about how to identify this problem and deal with it.
Suggested outline.
AcceptWebSocketAsync lives only as long as the Middleware pipeline for that request is still running. Once the middleware pipeline ends, the socket becomes broken.Task.Wait() or similar blocking calls to wait for the socket to complete, it will cause serious threading issues in the application, always use await.app.Use(async (context, next) => {
var socket = await context.WebSockets.AcceptWebSocketAsync();
var socketFinishedTcs = new TaskCompletionSource<object>();
// The background processor should call socketFinishedTcs.TrySetResult(null) when it has finished with the socket
BackgroundSocketProcessor.AddSocket(socket, socketFinishedTcs);
// Wait for the BackgroundSocketProcessor to finish with the socket before concluding the request.
await socketFinishedTcs.Task;
});
@anurse
Suggest providing some sample exception messages, to help searchability when the exception occurs
Can you add these exceptions to this issue and I'll try to squeeze this issue into this sprint.
@tdykstra review and assign to PU or accept.
@Rick-Anderson Looks like there's enough to go on here to add the requested section to the doc. Sample error messages would be helpful, but if we don't have them we can go with what we have.
Can you add these exceptions to this issue
I don't have the latest message on-hand unfortunately. It's pretty easy to force this problem to occur by doing something like this:
app.Use(async (context, next) => {
var socket = await context.WebSockets.AcceptWebSocketAsync();
Task.Run(async () => {
await Task.Delay(1000);
try {
await socket.SendAsync(...); // This should throw the exception
} catch(Exception ex) {
// Put a breakpoint here and you should see the exception
}
});
});
The code starts a background task, ends the request and then 1 second later (after the request has ended) the background task writes to the socket and should throw the exception. I believe it should be an ObjectDisposedException. The message may be a little clearer now (I think we changed it to something like "Attempted to write to the response after the request ended")
I'm seeing a WebSocketException:
The remote party closed the WebSocket connection without completing the close handshake.
with ObjectDisposedException inner exception:
Cannot write to the response body, the response has completed.
Object name: 'HttpResponseStream'.
That's the one
A sample pattern for this is given below (I suggest expanding it to something more functional in the actual doc, I'm just putting a rough sketch in here right now)
@anurse Is the "rough sketch" code good enough to use now and plug in the "more functional" code later when it's available?
I think the rough sketch is actually fine as-is. No need to refine it further.
Looks like this issue was mostly taken care of by #12378
I can expand on that by adding the code sample from this issue.
Most helpful comment
I don't have the latest message on-hand unfortunately. It's pretty easy to force this problem to occur by doing something like this:
The code starts a background task, ends the request and then 1 second later (after the request has ended) the background task writes to the socket and should throw the exception. I believe it should be an
ObjectDisposedException. The message may be a little clearer now (I think we changed it to something like "Attempted to write to the response after the request ended")