Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

writeHeader can race and be called multiple times #15

Open
bendoerr opened this issue May 28, 2020 · 1 comment
Open

writeHeader can race and be called multiple times #15

bendoerr opened this issue May 28, 2020 · 1 comment

Comments

@bendoerr
Copy link

Because the headerWritten bool is both read and modified outside of the protection of the mutex multiple calls to writeHeader are needlessly made. I suggest moving the headerWritten bool onto the Header type and then inside writeHeader check if it's been set or exit.

@mitchellh
Copy link
Owner

mitchellh commented May 29, 2020

I haven't looked at this code in awhile, but how is headerWritten racy? I don't think the underlying http.ResponseWriter allows for any sort of concurrency, therefore the httpsnoop callbacks can't race either.

Also our second read of headerWritten is after ServeHTTP returns, and the net/http docs on ResponseWriter explicitly state that ResponseWriter calls must not be called after that point, so we can be safe there in reading the value and knowing we can't get a read/write race.

We have to protect writeHeader with a mutex because the actual go-server-timing metrics may be concurrently accessed. Actually, we expect them to be otherwise your server timing is probably just a waterfall. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants