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

Jump to bottom

net/http/httptest: Close does not wait for the underlying Server's ConnState callbacks to complete #37510

Closed
bcmills opened this issue Feb 27, 2020 · 4 comments
Closed

net/http/httptest: Close does not wait for the underlying Server's ConnState callbacks to complete #37510

bcmills opened this issue Feb 27, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 27, 2020

In issue #37505, the observed data race seems to be due to the http.Server embedded in an httptest.Server firing a ConnState callback after the call to the httptest.Sever's Close method has already returned.

The documentation for (*httptest.Server).Close says that it “blocks until all outstanding requests on this server have completed”, so it seems reasonable for users to assume that the corresponding ConnState callbacks on the corresponding connections will have completed by that point as well. Empirically, that does not always happen.

CC @bradfitz @rsc @tombergan
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 27, 2020
@bcmills bcmills added this to the Backlog milestone Feb 27, 2020
@bcmills bcmills mentioned this issue Jan 29, 2021
Closed
@bcmills bcmills mentioned this issue Mar 25, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/304829 mentions this issue: net/http/httptest: wait for user ConnState hooks

@bcmills
Copy link
Contributor Author

bcmills commented Nov 29, 2021

@gopherbot, please backport to Go 1.16. This may be the root cause of the test failure observed in #49850.

@gopherbot gopherbot mentioned this issue Nov 29, 2021
Closed
@gopherbot
Copy link

Backport issue(s) opened: #49851 (for 1.16).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot
Copy link

Change https://golang.org/cl/367516 mentions this issue: [release-branch.go1.16] net/http/httptest: wait for user ConnState hooks

gopherbot pushed a commit that referenced this issue Dec 1, 2021
@neild @mknyszek
[release-branch.go1.16] net/http/httptest: wait for user ConnState hooks
0f9838e 
Ensure that user ConnState callbacks have completed before returning
from (*httptest.Server).Close.

Fixes: #49851
Updates: #37510
Updates: #37505
Updates: #45237

Change-Id: I8fe7baa089fbe4f3836bf6ae9767c7b1270d1331
Reviewed-on: https://go-review.googlesource.com/c/go/+/304829
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 5cec8b8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/367516
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
@golang golang locked and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@bcmills @gopherbot