Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(18)

Issue 46570043: code review 46570043: net/http: fix data race with Transport.CancelRequest (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by bradfitz
Modified:
10 years, 2 months ago
Reviewers:
r
CC:
golang-codereviews, r
Visibility:
Public.

Description

net/http: fix data race when sharing request body between client and server A server Handler (e.g. a proxy) can receive a Request, and then turn around and give a copy of that Request.Body out to the Transport. So then two goroutines own that Request.Body (the server and the http client), and both think they can close it on failure. Therefore, all incoming server requests bodies (always *http.body from transfer.go) need to be thread-safe. Fixes Issue 6995

Patch Set 1 #

Patch Set 2 : diff -r 119aee4343a6 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 119aee4343a6 https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r 1b7c5daffdff https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r 1b7c5daffdff https://go.googlecode.com/hg/ #

Total comments: 3

Patch Set 6 : diff -r b12d73776439 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -13 lines) Patch
M src/pkg/net/http/response_test.go View 1 2 3 3 chunks +5 lines, -2 lines 0 comments Download
M src/pkg/net/http/serve_test.go View 1 2 3 4 5 1 chunk +58 lines, -0 lines 0 comments Download
M src/pkg/net/http/transfer.go View 1 2 3 7 chunks +33 lines, -8 lines 0 comments Download
M src/pkg/net/http/transfer_test.go View 1 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 7
bradfitz
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
10 years, 3 months ago (2013-12-31 00:15:25 UTC) #1
r
LGTM
10 years, 2 months ago (2013-12-31 19:19:07 UTC) #2
bradfitz
Hello golang-codereviews@googlegroups.com, r@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
10 years, 2 months ago (2014-01-06 23:54:12 UTC) #3
bradfitz
PTAL Now with test, actual understanding of the issue (see CL description + test body), ...
10 years, 2 months ago (2014-01-06 23:56:31 UTC) #4
r
LGTM https://codereview.appspot.com/46570043/diff/80001/src/pkg/net/http/serve_test.go File src/pkg/net/http/serve_test.go (right): https://codereview.appspot.com/46570043/diff/80001/src/pkg/net/http/serve_test.go#newcode2098 src/pkg/net/http/serve_test.go:2098: // Therefore, all incoming server requests Bodies need ...
10 years, 2 months ago (2014-01-07 18:25:49 UTC) #5
bradfitz
All done. On Tue, Jan 7, 2014 at 10:25 AM, <r@golang.org> wrote: > LGTM > ...
10 years, 2 months ago (2014-01-07 18:40:57 UTC) #6
bradfitz
10 years, 2 months ago (2014-01-07 18:41:00 UTC) #7
*** Submitted as https://code.google.com/p/go/source/detail?r=00cce3a34d7e ***

net/http: fix data race when sharing request body between client and server

A server Handler (e.g. a proxy) can receive a Request, and
then turn around and give a copy of that Request.Body out to
the Transport. So then two goroutines own that Request.Body
(the server and the http client), and both think they can
close it on failure.  Therefore, all incoming server requests
bodies (always *http.body from transfer.go) need to be
thread-safe.

Fixes Issue 6995

R=golang-codereviews, r
CC=golang-codereviews
https://codereview.appspot.com/46570043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b