net/http: Request not cancelled when body is not read

[sheerun]

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN="/Users/sheerun/go/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/sheerun/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/22/xz6_9gpx3jggts_8j68_25g80000gn/T/go-build547040878=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

I wanted proper go-lang server that detects when request is cancelled:

package main

import (
	"fmt"
	"net/http"
	"time"
)

func Handler(w http.ResponseWriter, req *http.Request) {
	go func() {
		<-req.Context().Done()
		fmt.Println("done")
	}()

	time.Sleep(10 * time.Second)
}

func main() {
	http.ListenAndServe(":8080", http.HandlerFunc(Handler))
}

What did you expect to see?

Request is immediately cancelled for following calls:

curl http://localhost:8080/hello # then Ctrl+C
curl http://localhost:8080/hello -X POST -d 'asdfa' # then Ctrl+C
curl http://localhost:8080/hello -X PUT  -d 'foobar' # then Ctrl+C

What did you see instead?

Request is not cancelled for POST and PUT with body.

Side note: when POST of PUT is sent without body, the request is cancelled immediately as expected

curl http://localhost:8080/hello -X POST  # then Ctrl+C
curl http://localhost:8080/hello -X PUT # then Ctrl+C

[ianlancetaylor]

CC @bradfitz @tombergan

[sheerun]

I'll only mention it isn't issue with CURL as, following equivalent in node.js works without issues even for POST requests with body:

const express = require('express')
const app = express()

app.all('/', (req, res) => {
  req.connection.on('close', function (){
    console.log('done');
  });
  setTimeout(function () {
    res.send('Hello World!')
  }, 10000);
})

app.listen(3000, () => console.log('Example app listening on port 3000!'))

[sheerun]

Another note, when I use req.Body.Close() it behaves properly:

package main

import (
	"fmt"
	"net/http"
	"time"
)

func Handler(w http.ResponseWriter, req *http.Request) {
	req.Body.Close()

	go func() {
		<-req.Context().Done()
		fmt.Println("done")
	}()

	time.Sleep(10 * time.Second)
}

func main() {
	http.ListenAndServe(":8080", http.HandlerFunc(Handler))
}

But in my use case I don't want to close the body and still handle request cancellations (specifically: I want to stream request body after unknown time, but allow to cancel request from client side, a form of file p2p file transfering).

[sheerun]

Also, it seems because of this issue WriteTimeout setting of http.Server has no effect for POST/PUT queries with body (unless manually closing body, what misses the point of this flag).

[sheerun]

@bradfitz @tombergan I'm struggling to find workaround, any help?

[bradfitz]

The HTTP request context only expires once the Request.Body has been fully consumed.

Maybe we just need to document this.

If we could get TCP details out of the operating system portably and see when a connection got a RST before actually reading our way to it, that'd be nice, but I don't think /proc/net/tcp on Linux even contains that bit. Or maybe it's in "connection state". @tombergan, do you know?

[sheerun]

I strongly believe it is not something to document but to fix or at least provide alternative for.
This behaviour makes it impossible to implement http streaming with client-side cancelling in go.

[sheerun]

Plus it's certainly possible because as mentioned express in node.js has it

[bradfitz]

Okay, how? How does NodeJs do it?

[sheerun]

My quick research:

It starts here: https://github.com/nodejs/node/blob/48813dee2794e2f6993b72655a8dbd3011a908ab/lib/net.js#L552-L555
Which leads to: https://github.com/nodejs/node/blob/master/src/tcp_wrap.cc#L90
Which leads to: https://github.com/nodejs/node/blob/0a842633861cf20527ce93e6bf58b4ece1b168be/src/handle_wrap.cc#L63-L84

And that leads to:

Windows implementation: https://github.com/libuv/libuv/blob/2b32e77bb6f41e2786168ec0f32d1f0fcc78071b/src/win/tcp.c#L1376-L1447

Unix implementation:
https://github.com/libuv/libuv/blob/2b32e77bb6f41e2786168ec0f32d1f0fcc78071b/src/unix/stream.c#L1637-L1688

[bradfitz]

None of that code looks relevant to this issue.

Instead, show me a Node JS program that accepts a POST request, doesn't read its body, and gets notified when the connection goes away.

[sheerun]

I've already posted it: #23262 (comment)