Timed out poll connections kill the connection

[moozzyk]

If a poll connection times out the server throws:

System.InvalidOperationException occurred
  HResult=-2146233079
  Message=Concurrent reads are not supported.
  Source=System.Private.CoreLib
  StackTrace:
       at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Channels.Channel.Channels.IReadableBufferAwaiter.GetResult()
       at Microsoft.AspNetCore.Sockets.LongPolling.<ProcessRequestAsync>d__3.MoveNext() in C:\source\NetworkingProtoype\src\Microsoft.AspNetCore.Sockets\LongPolling.cs:line 21
  InnerException: 

and kills the connection. The client restarts the poll only to get:

GET http://localhost:5000/hubs/poll?id=87085e15-6bb5-492e-850b-f7965c7e9bbc&87085e15-6bb5-492e-850b-f7965c7e9bbc 500 (Internal Server Error)

[davidfowl]

This means the previous poll is still going. We need to define the behavior when a request is made to the same "connection". Do we reject the request or replace the previous one.

Old SignalR replaced the previous one.

[moozzyk]

We need to replace the previous one because it is the default client behavior - cancel a request after 110 secs of inactivity and start a new poll.

[davidfowl]

We do have to replace.

We need to replace the previous one because it is the default client behavior

That's not right. The server should end the response, not the client.

[moozzyk]

The client has to end long running requests otherwise IIS will terminate after 120 secs. That's why we have the 110 sec timeout on the client - or I am missing something.

[moozzyk]

or we move the timeout logic to the server somehow and we write 0 bytes after the timeout and the client restarts the poll

[davidfowl]

or we move the timeout logic to the server somehow and we write 0 bytes after the timeout and the client restarts the poll

This is what old SignalR does. Do you think we should do the replace behavior for more than just long polling? I'm thinking we return a 409 conflict for other requests... (sse and websockets)

[moozzyk]

The old SignalR timeouts polls on the client.
There is no question that for sse and websockets we should reject a connect request if there is already an opened stream/websocket for this connectio id. For longpolling we may try handling reconnects by require the server to stop the poll requests but I am afraid of cases where something (e.g. a proxy) on the way terminates the current poll request and the client will try to start a new poll which will be rejected even though in theory it could/should just work.

[davidfowl]

There is no question that for sse and websockets we should reject a connect request if there is already an opened stream/websocket for this connectio id

I have a feeling that we'll end up supporting reconnect everywhere after doing this.

It's server side https://github.com/SignalR/SignalR/blob/1fba14fa3437e24c204dfaf8a18db3fce8acad3c/src/Microsoft.AspNet.SignalR.Core/Transports/TransportHeartBeat.cs#L213-L217. The client side has a timeout as well but during normal operation it's never triggered.

[davidfowl]

Fixed