Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: net/http: Server.ConnContext should be invoked *after* forking go c.serve(...) #64837

ahmedtd opened this issue Dec 21, 2023 · 3 comments


Copy link

ahmedtd commented Dec 21, 2023

Proposal Details

Code link:;l=3077

Today, when an http.Server has a ConnContext function set, ConnContext is called inline in the accept loop, before forking off a separate goroutine to run the HTTP serving logic.

If ConnContext takes time, this ends up driving tail latency for all incoming connections in the accept queue behind the slow invocation. We originally hit this while investigating high latency in GKE and ASM when using GKE Workload Identity; the flow involves a ConnContext function that looks up callers by IP to authenticate them).

If ConnContext were invoked as the first step after forking the goroutine, then long-running ConnContext calls would not block accept() for other incoming connections. This would technically be a behavior change, which is why I'm submitting a proposal, but I don't really see how it could cause a problem.

@gopherbot gopherbot added this to the Proposal milestone Dec 21, 2023
Copy link

Jorropo commented Dec 21, 2023

It seems to me that both features are useful, having it inline in the loop allows to apply back-pressure and avoid runaway memory usage.

Let's assume someone is DOSing you, and you are doing IP based rate limiting inside ConnContext.
If this is done after the fork you may have a transient where the attacker can spawn thousands of goroutines before the rate limiting code is being executed.

Someone wary of that could also implement this by wrapping net.Listener object and doing inline validation in the .Accept method.

Copy link

ahmedtd commented Dec 22, 2023

Good point... Maybe there should be a separate PostForkConnContext function.

Copy link

bcmills commented Jan 2, 2024

(attn @neild)

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

No branches or pull requests

4 participants