Skip to content

fix(ext/node): drain keep-alive connections on http server.close()#35614

Open
bartlomieju wants to merge 1 commit into
mainfrom
fix/http-server-close-drain-keepalive
Open

fix(ext/node): drain keep-alive connections on http server.close()#35614
bartlomieju wants to merge 1 commit into
mainfrom
fix/http-server-close-drain-keepalive

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Deno's node:http server.close() diverged from Node. Per Node semantics,
server.close() stops accepting new connections but keeps existing keep-alive
connections open so in-flight and subsequent requests on them finish; the close
callback only fires once those connections end. Under Deno, server.close()
tore down the existing keep-alive connection as soon as the in-flight response
finished, so a follow-up request on the same socket was never served and the
close callback fired immediately.

The cause was a Deno-only branch in resOnFinish that destroyed the socket
when the server was no longer listening, instead of setting the keep-alive
timeout. This branch was introduced in the llhttp rewrite (#33208) with a note
about preventing timer leaks, but it breaks the graceful-drain behavior that
frameworks like fastify rely on for graceful shutdown (serving HTTP 503 to
in-flight connections while closing). Removing it makes the connection drain
like Node: the keep-alive timeout (or a Connection: close request) eventually
ends the socket.

Adds a regression test in tests/unit_node/http_test.ts that calls
server.close() during the first request and asserts a second request on the
same keep-alive socket is still served.

Closes #35609

`server.close()` tore down existing keep-alive connections as soon as the
in-flight response finished, so a follow-up request on the same socket was
never served and the close callback fired immediately. Node keeps existing
keep-alive connections open so in-flight and subsequent requests on them
finish, and only fires the close callback once those connections end.

`resOnFinish` had a Deno-only branch that destroyed the socket when the
server was no longer listening. Remove it so the keep-alive timeout is set
like Node, letting the connection drain.

Closes #35609
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant