-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
http2: fix Server race with concurrent Read/Close
Introduced by golang.org/cl/31447. grpc-go does concurrent Read/Close calls to the Request.Body. Since that was supported previously, continue to support it, and add a test. Updates grpc/grpc-go#938 Change-Id: I8a5cf6b28c5eca346ea46c4129021172e489f71d Reviewed-on: https://go-review.googlesource.com/31636 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
- Loading branch information
1 parent
daba796
commit 40a0a18
Showing
4 changed files
with
36 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ var ( | |
VerboseLogs bool | ||
logFrameWrites bool | ||
logFrameReads bool | ||
inTests bool | ||
) | ||
|
||
func init() { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40a0a18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradfitz it appears that two concurrent calls to Close() can still race here because b.closed is being checked and set in two separate operations (i.e. a check, check, set, set ordering is possible). Is this commit intended to support concurrent calls to Close()?
40a0a18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@voutasaurus Probably best to open a new issue that references this commit sha.
https://github.com/golang/go/issues/new
40a0a18
(otherwise your comment might get lost)
40a0a18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Is there code in the wild trying to concurrently close?
40a0a18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, what @nathany said. We don't use comments on github issues for anything.