Jetty WebSocket Session.suspend() not implemented

[joakime]

The Jetty WebSocket Session.suspend() API is unimplemented in Jetty 10.0.x

[gregw]

Was this ever fully implemented? Used?

If so, it should be able to be mapped to the demand mechanism, but may be a little complex as we don't really want to always be demandable just in case somebody calls suspend. My preference would be to not implement suspend and instead expose demand directly

[joakime]

This is fully implemented in Jetty 9.4.x and we've even had issues and PR submitted from the community around it too.

[gregw]

@sbordet I've seen the changes you have suggested to @lachlan-roberts and I don't agree with them.

Suspend is an intrinsically broken semantic as there is nothing we can reasonably do to prevent the delivery of a frame after a call to suspend. Suspend is fundamentally a race by design and thus has no place being implemented in core. Core has the demand reactive mechanism which is currently seen as a best practise for predictably handling asynchronous back pressure. If we wish to support suspend in the jetty API, then it needs to be implemented in terms of demand and not by hacking a specific imperfect semantic into the core connection class - that already supports a well designed mechanism. Sure using demand means that you cannot issue negative demand, so you can't prevent a message from being delivered after a suspend call, but you can't do that anyway even with a hack in the connection class. If the jetty API wishes to reduce the size of the race window, it is free to hold a single frame that arrives after a suspend.

[gregw]

... and just to be more specific about what is broken about the suspend API....

No matter how it is implemented, if suspend can be called outside the scope of frame handling call, it cannot prevent a frame being delivered to an application after suspend is called.

Imagine an application that is calling suspend because it has reached some resource limit - it is out of threads, memory, DB connections etc. If it calls suspend from an arbitrary thread, then no matter what implementation we have, another thread may already have been dispatched to deliver a frame to the app and those resource limits may be blown! If we implement suspend in the lower layers, then the window for this to happen is small, but it still exists and we can still break an application by delivering a frame after suspend is called.

The only way we can guarantee to an application that we will not blow up its suspend policed resource limits is if suspend is called from within a frame handling thread. This is true if we implement suspend with demand or if we implement it in the core connection. OK the window for the race to happen with demand is large and the window for the connection impl is small, but in either case the race still exists and the contract for suspend should be documented that calling it from an arbitrary thread may not stop 1 more frame from being delivered. If we are really worried about the size of the window with the demand implementation, we can always make the JettyWebSocketFrameHandler class hold a single frame if it is delivered when suspended and the delivery can be completed once resume is called. This doesn't close the race window, it just makes it small again.

To repeat, a fundamentally racy mechanism like suspend should not be implemented as part of core. If the semantic is desired, then it can be achieved above core.

[sbordet]

@gregw I don't follow.

We use callbacks in the client, in the server, in http/2 to backpressure properly, from any thread.
We don't move forward unless the previous callback is completed.

Are you telling me that core is not designed in this way, but rather in a broken way that now does not support suspend, nor calling a callback from another thread, nor possibly support proper backpressure?

[lachlan-roberts]

The Jetty WebSocket Session.suspend() API has now been implemented in Jetty 10.0.x and is tested in SuspendResumeTest