You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
To preface, the change in the signature makes complete sense from a resource safety perspective. IIRC the former definition is actually a little bit misleading; the socket associated with the inner Resource has already been allocated before use is even called! As a consequence, if use is not called, the socket is never closed and gets stuck in limbo, resulting in a file leak.
The larger issue here is that the lifecycles of the server socket and all accepted sockets are now bound to the same stream. However, in practice, it's completely sound for them to have disjoint lifecycles e.g. you can close a server socket while leaving sockets that were accepted open.
Ember Server exploited the lack of safety in the old definition to implement graceful shutdowns: we can interrupt the server stream and force closure upon it while allowing existing connections to complete their work for some time. parJoin semantics are actually too strict because inner streams lease the input stream's scope and prevents its resources from finalizing until all inner streams are closed. So we implemented a weaker version of parJoin called forking that doesn't lease any scopes, which is unsafe in general but completely fine for this particular use case.
forking completely breaks with the new definition of serverResource precisely because it doesn't lease the scope. Since the accepted sockets are bound to the server stream, as soon as a connection stream is spawned, the socket is going to finalize. Is there a way to only partially lease some resources from the server stream in a safe manner? In particular, lease the accepted socket but not the server socket.
This is blocking Http4s builds on CE3 ATM, and reverting to parJoin would be a pretty major regression in behavior (and performance to a lesser degree), so it would be cool if we could come to a resolution on this soon
The text was updated successfully, but these errors were encountered:
The larger issue here is that the lifecycles of the server socket and all accepted sockets are now bound to the same stream.
Ok so this is actually not true, as evidenced by the signature. The server resource is the outer resource and the client sockets are on the inner stream. However, Ember Server (and others I imagine) forcibly bind it to the same stream via Stream.resource(...).flatMap(...). I'm going to see if untangling that works, but I have a feeling that it won't, in which case I have another idea to make the old Resource signature safe
On 2.5.x, the return type for
serverResource
is:On the latest 3.x milestone, the return type is now:
To preface, the change in the signature makes complete sense from a resource safety perspective. IIRC the former definition is actually a little bit misleading; the socket associated with the inner
Resource
has already been allocated beforeuse
is even called! As a consequence, ifuse
is not called, the socket is never closed and gets stuck in limbo, resulting in a file leak.The larger issue here is that the lifecycles of the server socket and all accepted sockets are now bound to the same stream. However, in practice, it's completely sound for them to have disjoint lifecycles e.g. you can close a server socket while leaving sockets that were accepted open.
Ember Server exploited the lack of safety in the old definition to implement graceful shutdowns: we can interrupt the server stream and force closure upon it while allowing existing connections to complete their work for some time.
parJoin
semantics are actually too strict because inner streams lease the input stream's scope and prevents its resources from finalizing until all inner streams are closed. So we implemented a weaker version ofparJoin
called forking that doesn't lease any scopes, which is unsafe in general but completely fine for this particular use case.forking
completely breaks with the new definition ofserverResource
precisely because it doesn't lease the scope. Since the accepted sockets are bound to the server stream, as soon as a connection stream is spawned, the socket is going to finalize. Is there a way to only partially lease some resources from the server stream in a safe manner? In particular, lease the accepted socket but not the server socket.This is blocking Http4s builds on CE3 ATM, and reverting to
parJoin
would be a pretty major regression in behavior (and performance to a lesser degree), so it would be cool if we could come to a resolution on this soonThe text was updated successfully, but these errors were encountered: