From 4f0bc49a84e31b6a0a8b348400a385cb2e9b91a1 Mon Sep 17 00:00:00 2001 From: Kim De Mey Date: Thu, 12 Sep 2024 10:47:02 +0200 Subject: [PATCH] Remove the early-fin and add a 4s timeout on socket destroy instead (#2614) --- fluffy/network/wire/portal_stream.nim | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/fluffy/network/wire/portal_stream.nim b/fluffy/network/wire/portal_stream.nim index f7cfd3b2a..93d897d5a 100644 --- a/fluffy/network/wire/portal_stream.nim +++ b/fluffy/network/wire/portal_stream.nim @@ -248,11 +248,24 @@ proc readContentOffer( await socket.destroyWait() else: # This means FIN didn't arrive yet, perhaps it got dropped but it might also - # be still in flight. Closing the socket (= sending FIN) ourselves. - # Not waiting here for its ACK however, so no `closeWait`. Underneath the - # socket will still wait for the FIN-ACK (or timeout) before it destroys the - # socket. - socket.close() + # be still in flight. + # + # uTP has one-way FIN + FIN-ACK to destroy the connection. The stream + # already has the information from the application layer to know that all + # required data was received. But not sending a FIN from our side anyhow as + # there is probably one from the other side in flight. + # Sending a FIN from our side turns out to not to improve the speed of + # disconnecting as other implementations seems to not like the situation + # of receiving our FIN before our FIN-ACK. + # We do however put a limited timeout on the receival of the FIN and destroy + # the socket otherwise. + proc delayedDestroy( + socket: UtpSocket[NodeAddress], delay: Duration + ) {.async: (raises: [CancelledError]).} = + await sleepAsync(delay) + await socket.destroyWait() + + asyncSpawn socket.delayedDestroy(4.seconds) # TODO: This could currently create a backlog of content items to be validated # as `AcceptConnectionCallback` is `asyncSpawn`'ed and there are no limits