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

Issue 86740044: code review 86740044: net/http: quiet useless warning during shutdown (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years ago by bradfitz
Modified:
10 years ago
Reviewers:
dsymonds
CC:
golang-codereviews, dsymonds, adg, rsc
Visibility:
Public.

Description

net/http: quiet useless warning during shutdown What was happening on Issue 7010 was handler intentionally took 30 milliseconds and the proxy's client timeout was 35 milliseconds. Then it slammed the proxy with a bunch of requests. Sometimes the server would be too slow to respond in its 5 millisecond window and the client code would cancel the request, force-closing the persistConn. If this came at the right time, the server's reply was already in flight, and one of the goroutines would report: Unsolicited response received on idle HTTP channel starting with "H"; err=<nil> ... rightfully scaring the user. But the error was already handled and returned to the user, and this connection knows it's been shut down. So look at the closed flag after acquiring the same mutex guarding another field we were checking, and don't complain if it's a known shutdown. Also move closed down below the mutex which guards it. Fixes Issue 7010

Patch Set 1 #

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

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

Total comments: 1

Patch Set 4 : diff -r 68f19a123155 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -7 lines) Patch
M src/pkg/net/http/transport.go View 1 2 3 3 chunks +9 lines, -7 lines 0 comments Download

Messages

Total messages: 3
bradfitz
Hello golang-codereviews@googlegroups.com (cc: adg@golang.org, dsymonds@golang.org, rsc@golang.org), I'd like you to review this change to https://go.googlecode.com/hg/
10 years ago (2014-04-11 06:40:21 UTC) #1
dsymonds
LGTM well, that was easy https://codereview.appspot.com/86740044/diff/40001/src/pkg/net/http/transport.go File src/pkg/net/http/transport.go (right): https://codereview.appspot.com/86740044/diff/40001/src/pkg/net/http/transport.go#newcode777 src/pkg/net/http/transport.go:777: defer pc.lk.Unlock() defer here ...
10 years ago (2014-04-11 06:58:09 UTC) #2
bradfitz
10 years ago (2014-04-11 16:40:42 UTC) #3
*** Submitted as https://code.google.com/p/go/source/detail?r=fab302d4e9c4 ***

net/http: quiet useless warning during shutdown

What was happening on Issue 7010 was handler intentionally took 30
milliseconds and the proxy's client timeout was 35 milliseconds. Then it
slammed the proxy with a bunch of requests.

Sometimes the server would be too slow to respond in its 5 millisecond
window and the client code would cancel the request, force-closing the
persistConn.  If this came at the right time, the server's reply was
already in flight, and one of the goroutines would report:

Unsolicited response received on idle HTTP channel starting with "H"; err=<nil>

... rightfully scaring the user.

But the error was already handled and returned to the user, and this
connection knows it's been shut down. So look at the closed flag after
acquiring the same mutex guarding another field we were checking, and
don't complain if it's a known shutdown.

Also move closed down below the mutex which guards it.

Fixes Issue 7010

LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://codereview.appspot.com/86740044
Sign in to reply to this message.

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