Streaming body issue with a synchronous ring handler

[chpill]

There is an issue with streaming responses when using a synchronous ring handler. Consider the following bare-bone SSE handler:

(Tested with Clojure 1.12.0-alpha5 and Java 21)

{:deps {org.clojure/clojure {:mvn/version "1.12.0-alpha5"}
        ring/ring-jetty-adapter {:mvn/version "1.11.0"}}}

(ns sse-with-ring
  (:require [clojure.java.io :as io]
            [ring.adapter.jetty :refer [run-jetty]]
            [ring.core.protocols]))

(defn handler [_]
  {:status 200
   :body (reify
           ring.core.protocols/StreamableResponseBody
           (write-body-to-stream [_ _ output-stream]
             (let [writer (io/writer output-stream)]
               (future (loop [i 10]
                         (doto writer (.write (str "data: " i "\n")) .flush)
                         (if (zero? i)
                           (.close writer)
                           (do (Thread/sleep 100)
                               (recur (dec i)))))))))})

(def server-async (run-jetty (fn [req respond raise]
                               (try (respond (handler req))
                                    (catch Exception e (raise e))))
                             {:port 12345
                              :async? true
                              :join? false}))

(comment (.stop server-async))

(def server-sync (run-jetty (fn [req] (handler req))
                            {:port 23456
                             :join? false}))

(comment (.stop server-sync))

Calling the async server with curl yields the expected result, streaming a message every 100ms and finally closing:

curl -Nv localhost:12345
*   Trying [::1]:12345...
* Connected to localhost (::1) port 12345
> GET / HTTP/1.1
> Host: localhost:12345
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Wed, 10 Jan 2024 18:04:25 GMT
< Transfer-Encoding: chunked
< Server: Jetty(11.0.18)
<
data: 10
data: 9
data: 8
data: 7
data: 6
data: 5
data: 4
data: 3
data: 2
data: 1
data: 0
* Connection #0 to host localhost left intact

But calling the synchronous server returns instantly without displaying any of the "data: x" messages.

curl -Nv localhost:23456
*   Trying [::1]:23456...
* Connected to localhost (::1) port 23456
> GET / HTTP/1.1
> Host: localhost:23456
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Wed, 10 Jan 2024 18:04:39 GMT
< Content-Length: 0
< Server: Jetty(11.0.18)
<
* Connection #0 to host localhost left intact

The response should behave the same way whether the :async true setting is present or not.

(This issue was originally encountered while reporting another issue in the ring-jetty9-adapter project)

[weavejester]

Initially I thought this was a bug, but now that I've investigated it more fully, I don't believe it is.

The reason this doesn't work with synchronous handlers is because the adapter expects the write-body-to-stream protocol method to also be synchronous. That is, the response is complete when both the handler and write-body-to-stream have completed.

This seems like reasonable behavior to have, as synchronous functions have guarantees that asynchronous ones do not. A thread is assigned to the handler, and then to write the body stream, and then given back up to the pool. There's no instance where the body stream could be left open without also blocking the request thread.

Going back to the handler function you gave as an example, we could rewrite this to work correctly just by removing the future. The only case where this doesn't work is if we want to write data asynchronously to the output stream, without blocking a worker thread; but this is what :async? true is for.

My current thinking is that it's better to keep the guarantees that synchronous, blocking I/O give us. Particularly if Java's virtual threads allow us to park a thread for relatively little cost in future.

[chpill]

My current thinking is that it's better to keep the guarantees that synchronous, blocking I/O give us. Particularly if Java's virtual threads allow us to park a thread for relatively little cost in future.

I think I understand your point, this seems simpler and safer.

I'm re-reading the doc string of the protocol now:

  (write-body-to-stream [body response output-stream]
    "Write a value representing a response body to an output stream. The stream
    will be closed after the value had been written. The stream may be written
    asynchronously.")

In retrospect, maybe "after the value has been written" means "after write-body-to-stream returns"? Which would then only true in the "ring sync" case. I guess this is quite tricky to document because details from the implementation seem to leak into the protocol. The lifecycle of output-stream is managed by ring sync, but left completely up to the user by ring async.

[weavejester]

Yes, I think we want to update the protocol docstring to explain the use-case more clearly.

[sunng87]

Back to servlet spec, it does not allow async thread to write to HttpServletResponse outside handler methods of HttpServlet. Jetty has flexible requirement for this but it requires a callback to be resolved when write finished. For now, it's recommended to use :async ring handlers for this I think.