Closed chpill closed 3 months ago
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.
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.
Yes, I think we want to update the protocol docstring to explain the use-case more clearly.
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.
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)
Calling the async server with curl yields the expected result, streaming a message every 100ms and finally closing:
But calling the synchronous server returns instantly without displaying any of the "data: x" messages.
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)