Akka-http: Streamed response not immediately completed when client disconnects

[francisdb]

Akka http 10.1.0

Reproducer below

                get {
                  complete {
                    logger.info("start")
                    Source
                      .tick(60.seconds, 60.seconds, NotUsed)
                      .map(_ => ServerSentEvent("test"))
                      .watchTermination() { (m, f) =>
                        f.onComplete(r => logger.info(r.toString))
                        m
                      }
                   }
                 }

Now make a very short connection

curl http://localhost:8090/test
# once the connection is made press ctrl-c
^C

You will see that it takes 120 seconds for the stream to complete

13:49:02.390 INFO  - Foo - start
13:51:02.414 INFO  - Foo - Success(Done)

This could lead to a resource leak if nothing else is going to be sent over the stream?

[johanandren]

It is actually unrelated to SSE, just streaming a response and cancelling early with curl leads to the same behavior. I'll investigate a bit more.

[harpocrates]

The following diff fixes it, but I'm not 100% sure this doesn't accidentally re-introduce some deadlock (I did not understand all the details around f32a0d35a5bbe4ee38039035a3a2deb7b4aa2cb5).

diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala
index b769b007d..a8a3d61b6 100644
--- a/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala
+++ b/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala
@@ -91,9 +91,11 @@ private[http] class HttpResponseRendererFactory(
                 }
             }

-          override def onUpstreamFinish(): Unit =
-            if (transferring) closeMode = CloseConnection
-            else completeStage()
+          override def onUpstreamFinish(): Unit = {
+            closeMode = CloseConnection
+            stopTransfer()
+            completeStage()
+          }

           override def onUpstreamFailure(ex: Throwable): Unit = {
             stopTransfer()

In any case, the issue is caused by HttpResponseRendererFactory. In particular, the graph stage always waits for some element either from the input port or transfer sink input to close.

[raboof]

I think HTTP allows the scenario where the client closes (with FIN) the request when the server is still busy sending the response, so for that reason cutting the transfer short onUpstreamFinish would AFAICS be the wrong behavior.

In the ^C case mentioned above, however, I would expect the client to abort both 'sides' of the connection with RST. In that case indeed we should abort the stream instead of running it to completion.

[jrudolph]

On Linux, when I interrupt curl with Ctrl+C, a FIN is sent but no RST and the socket is put into FIN_WAIT2 state.

I guess, the socket is kept around for a while for the usual reason that a completely closed socket will answer any incoming packets to that socket with RST. So, given an application that sends some data on a socket and then shuts down, you would like that data to still reach the other end. That can only be ensured if the socket is kept around until the other side has done its part of the TCP closing handshake.

On the server side that means we see a completion on the network input side but don't react to it. That's probably because in theory we support half-open connections where the response is streamed while the request is still coming in (as @raboof said above).

RFC 7260 Section 6.6 says this:

A client that receives a "close" connection option MUST cease sending
requests on that connection and close the connection after reading
the response message containing the "close"; if additional pipelined
requests had been sent on the connection, the client SHOULD NOT
assume that they will be processed by the server.

So this recommends clients to keep the connection open until after the response has been read.

So, I'd say, at least under a flag, we should support a behavior that propagates closing information and cancels an ongoing response entity when the network is closed prematurely.

All that said, without keep-alives there's no way to detect that a peer has "just gone away", i.e. it still has a routeable IP address but doesn't answer packets any more.

[jrudolph]

And that's also why we have the idle-timeouts as a last resort to close a connection where no data flows.