haskell-distributed / network-transport-tcp

TCP Realisation of Network.Transport
http://haskell-distributed.github.com
BSD 3-Clause "New" or "Revised" License
30 stars 27 forks source link

Time out new socket connections #53

Closed avieth closed 7 years ago

avieth commented 7 years ago

New connections will time out if the peer does not send the required handshake data promptly, i.e. an EndPointId followed by an EndPointAddress.

The transportConnectTimeout parameter from TCPParameters is re-used here, but maybe a new parameter should be defined.

avieth commented 7 years ago

Is it safe to use mask like this

I think it's safe, since I put the io within the restore. It'll have the same masking state as the call to withTimeout (forked threads inherit the masking state). I wanted to express this using bracket (which amounts to the same thing) but it seemed a bit awkward... maybe I just need to give it more thought.

avieth commented 7 years ago

~Oh I see what you mean. If forkServer is called with exceptions masked then yes, we'll have them masked here too. Could be a problem I guess.~ Never mind, forkServer uses forkIOWithUnmask to ensure that the request handler doesn't have them masked. Should be OK. But if we want to export it for use in arbitrary places then yes, we'll want to forkIOWithUnmask/unsafeUnmask.

avieth commented 7 years ago

Another place where a timeout could be considered: closing the socket. If we send CloseSocket but the peer never responds, we'll never close the socket. So it might be good to close the socket if no more data is received before a timeout. But then again, we're at the mercy of the peer in any case. They can always hold the socket open by creating a new lightweight connection, doing nothing, and when we try to close the socket, creating another one. I think in a case like this, the decision to force close the socket can only be made by the application.

facundominguez commented 7 years ago

Thanks @avieth.