net/http: make Server Handler's Request.Context be done on conn errors

This CL changes how the http1 Server reads from the client.

The goal of this change is to make the Request.Context given to Server
Handlers become done when the TCP connection dies (has seen any read
or write error). I didn't finish that for Go 1.7 when Context was
added to http package.

We can't notice the peer disconnect unless we're blocked in a Read
call, though, and previously we were only doing read calls as needed,
when reading the body or the next request. One exception to that was
the old pre-context CloseNotifier mechanism.

The implementation of CloseNotifier has always been tricky. The past
few releases have contained the complexity and moved the
reading-from-TCP-conn logic into the "connReader" type. This CL
extends connReader to make sure that it's always blocked in a Read
call, at least once the request body has been fully consumed.

In the process, this deletes all the old CloseNotify code and unifies
it with the context cancelation code. The two notification mechanisms
are nearly identical, except the CloseNotify path always notifies on
the arrival of pipelined HTTP/1 requests. We might want to change that
in a subsequent commit. I left a TODO for that. For now there's no
change in behavior except that the context now cancels as it was
supposed to.

As a bonus that fell out for free, a Handler can now use CloseNotifier
and Hijack together in the same request now.

Fixes #15224 (make http1 Server always in a Read, like http2)
Fixes #15927 (cancel context when underlying connection closes)
Updates #9763 (CloseNotifier + Hijack)

Change-Id: I972cf6ecbab7f1230efe8cc971e89f8e6e56196b
Reviewed-on: https://go-review.googlesource.com/31173
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

src/net/http/http.go

@@ -6,6 +6,7 @@ package http
 
 import (
 	"strings"
+	"time"
 	"unicode/utf8"
 
 	"golang_org/x/net/lex/httplex"
@@ -15,6 +16,10 @@ import (
 // Transport's byte-limiting readers.
 const maxInt64 = 1<<63 - 1
 
+// aLongTimeAgo is a non-zero time, far in the past, used for
+// immediate cancelation of network operations.
+var aLongTimeAgo = time.Unix(233431200, 0)
+
 // TODO(bradfitz): move common stuff here. The other files have accumulated
 // generic http stuff in random places.
 

src/net/http/serve_test.go

@@ -4202,13 +4202,11 @@ func testServerRequestContextCancel_ServeHTTPDone(t *testing.T, h2 bool) {
 	}
 }
 
+// Tests that the Request.Context available to the Handler is canceled
+// if the peer closes their TCP connection. This requires that the server
+// is always blocked in a Read call so it notices the EOF from the client.
+// See issues 15927 and 15224.
 func TestServerRequestContextCancel_ConnClose(t *testing.T) {
-	// Currently the context is not canceled when the connection
-	// is closed because we're not reading from the connection
-	// until after ServeHTTP for the previous handler is done.
-	// Until the server code is modified to always be in a read
-	// (Issue 15224), this test doesn't work yet.
-	t.Skip("TODO(bradfitz): this test doesn't yet work; golang.org/issue/15224")
 	defer afterTest(t)
 	inHandler := make(chan struct{})
 	handlerDone := make(chan struct{})
@@ -4237,7 +4235,7 @@ func TestServerRequestContextCancel_ConnClose(t *testing.T) {
 
 	select {
 	case <-handlerDone:
-	case <-time.After(3 * time.Second):
+	case <-time.After(4 * time.Second):
 		t.Fatalf("timeout waiting to see ServeHTTP exit")
 	}
 }