Semantically breaking change in SocketGroup.serverResource
Nobody has claimed this yet.
- Dominant language
- Scala
- Stars
- 2.5k
- Forks
- 636
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 7
Description
On 2.5.x, the return type for serverResource is:
Resource[F, (InetSocketAddress, Stream[F, Resource[F, Socket[F]]])]
On the latest 3.x milestone, the return type is now:
Resource[F, (SocketAddress[IpAddress], Stream[F, Socket[F]])]
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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with SocketGroup.serverResource and the Ember server's ember-server/src/main/scala/org/http4s/ember/server/internal/StreamForking.scala, then compare the current resource scopes with parJoin semantics. Reproduce the accepted-socket finalization behavior on the CE3 milestone and determine whether safe partial leasing can preserve independent server and connection lifecycles.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- networking, stream-processing
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100