rabbitmq-dotnet-client icon indicating copy to clipboard operation
rabbitmq-dotnet-client copied to clipboard

`SocketFrameHandler`'s `WriteTimeout` not applicable for async writes

Open Turnerj opened this issue 1 year ago • 3 comments

Within SocketFrameHandler, it specifically uses a write timeout and sets this on both the NetworkStream's WriteTimeout property (which internally sets it on the socket) as well as the frame handler's own WriteTimeout (which also sets it on the socket). The problem isn't necessarily setting it but the fact that the socket's SendTimeout, according to Microsoft's documentation, does not apply on asynchronous sends - the same ones used within the write loop of the socket handler.

There is an issue tracking better socket timeouts which would be applicable once they are designed and merged into the runtime but for now, the WriteTimeout gives at best a false sense of security. I can't see a way to patch around the behaviour without changes in the runtime.

I know you'd want a specific action here for your GitHub issues so I'd probably suggest some amount of documentation somewhere - perhaps both in-code for the SocketFrameHandler (I'm happy to contribute this) but also anywhere else you feel is appropriate.

For what its worth, because you read from the socket synchronously, you don't have the same issue with ReadTimeout/ReceiveTimeout.

Turnerj avatar Oct 14 '22 04:10 Turnerj

I realised after posting this that SendHeader on SocketFrameHandler does do synchronous sends so it isn't entirely pointless to set, now just rather potentially inconsistent behaviour between the socket writes in the write loop and the send header behaviour.

Turnerj avatar Oct 14 '22 06:10 Turnerj

Nice catch. As we're hopefully moving the project over to System.IO.Pipelines this would actually matter.

stebet avatar Oct 14 '22 08:10 stebet