Skip to content

Set SO_LINGER to {true, 0} before the linger loop - #1715

Open
eagle-head wants to merge 1 commit into
ninenines:masterfrom
eagle-head:improve-linger-loop
Open

Set SO_LINGER to {true, 0} before the linger loop#1715
eagle-head wants to merge 1 commit into
ninenines:masterfrom
eagle-head:improve-linger-loop

Conversation

@eagle-head

@eagle-head eagle-head commented May 18, 2026

Copy link
Copy Markdown

Implements the linger fix discussed in #1672 and the related erlang/otp#9529.

When the connection process is killed during the application-level linger loop with outbound data still unacknowledged by the peer, the underlying socket can stay open and the OS port leaks. Setting SO_LINGER to {true, 0} right after shutdown(write) makes the OS send RST and release the connection immediately whenever the BEAM closes the port, instead of waiting on the half-closed state.

Applied in both cowboy_http:terminate_linger/1 and the mirrored block in cowboy_http2:terminate_linger/1.

Note on testing

An earlier revision included an end-to-end CT case to reproduce the leak, but as @essen pointed out it does not reliably fail without the fix, so it has been removed per his suggestion.

Why it couldn't be made deterministic: the case set {sndbuf, 2097152} so the 1 MB body would buffer in the kernel and the process would reach terminate_linger with the data still unacknowledged. On Linux, though, an explicit SO_SNDBUF is capped by net.core.wmem_max (212992 by default) and setting it explicitly also disables send-buffer autotuning, so the real buffer stays well under 1 MB. The body send then blocks inside cowboy_req:reply, leaving the process parked in the send rather than in the linger loop — so killing it behaves the same with or without the fix. The fix itself is straightforward, so this PR keeps just the setopts change.

Verified locally (Linux, OTP 28):

  • make ct-http — 35/35 ok
  • make ct-http2 — 18/18 ok

@essen

essen commented Jun 8, 2026

Copy link
Copy Markdown
Member

I am unable to make the test fail without the source changes, waiting on CI to see if it fails there.

@essen

essen commented Jun 8, 2026

Copy link
Copy Markdown
Member

Wait nevermind, CI has the source change so it won't fail there.

Can you tell me how you ran the test to confirm that it failed without the source code changes?

If the test doesn't fail before the source code change then we should just not have it. It's OK if the test is too difficult to create and the change is obvious.

Cowboy already runs an application-level linger loop after
shutdown(write) to drain the peer side. However, if the
connection process is killed while in or after that loop the
socket can stay open and the OS port leaks. See
erlang/otp#9529.

Setting SO_LINGER to {true, 0} once the FIN has been sent
forces the OS to send RST and release the socket whenever the
BEAM closes the port, including on abrupt process exit.
@eagle-head
eagle-head force-pushed the improve-linger-loop branch from 23d73f4 to 1089e6f Compare June 8, 2026 21:08
@eagle-head

Copy link
Copy Markdown
Author

You're right — I can't give you a dependable pre-fix failure recipe, so I've removed the test and kept just the setopts fix.

What was happening: the case set {sndbuf, 2097152} expecting the 1 MB body to buffer in the kernel so the process would reach terminate_linger with the data still unacked. But on Linux an explicit SO_SNDBUF is capped at net.core.wmem_max (212992 here → inet:getopts reports 425984) and setting it explicitly also disables send-buffer autotuning, so the real buffer is well under 1 MB. The body send blocks inside cowboy_req:reply, so the process is parked in the send — not in terminate_linger — when the test kills it; the {linger, {true, 0}} line never runs, so the kill behaves the same with or without the fix. The red→green I reported earlier was environment/timing dependent, not the mechanism the test describes.

So I've dropped terminate_linger_releases_socket_on_kill and the long_body_h handler; the PR is now just the two-line fix in cowboy_http/cowboy_http2. Happy to add the deterministic inet:getopts check instead if you'd prefer some coverage.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants