Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/net/http2: http client closes stream prematurely if used concurrently #25652

Closed
ernado opened this issue May 31, 2018 · 3 comments
Closed

x/net/http2: http client closes stream prematurely if used concurrently #25652

ernado opened this issue May 31, 2018 · 3 comments

Comments

@ernado
Copy link
Contributor

@ernado ernado commented May 31, 2018

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

go version devel +3c4d3bd Wed May 30 22:54:22 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

I'm using go from tip. The go1.10.2 is affected too.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ernado/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build180192421=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Put a simple http server behind nginx and tried to send http2 requests concurrently.

Sample client and server:

package main

import (
	"bytes"
	"flag"
	"fmt"
	"io"
	"io/ioutil"
	"log"
	"net"
	"net/http"
	"runtime"
	"sync"
	"time"
)

var (
	client     = flag.Bool("client", false, "client")
	addr       = flag.String("addr", "localhost:8544", "addr")
	url        = flag.String("url", "https://hostname", "url")
	concurrent = flag.Bool("concurrent", true, "concurrent")
)

func main() {
	flag.Parse()
	if !*client {
		fmt.Println("listening on", *addr)
		http.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) {
			io.Copy(ioutil.Discard, request.Body)
			request.Body.Close()
			writer.WriteHeader(http.StatusOK)
		})
		http.ListenAndServe(*addr, nil)
		return
	}
	fmt.Println("client to", *url)
	wg := new(sync.WaitGroup)
	var cDialer = &net.Dialer{
		Timeout: 10 * time.Second,
	}
	var cNetTransport = &http.Transport{
		DialContext:         cDialer.DialContext,
		TLSHandshakeTimeout: 10 * time.Second,
		MaxIdleConns:        20,
		MaxIdleConnsPerHost: 10,
		IdleConnTimeout:     60 * time.Second,
	}
	var cNetClient = &http.Client{
		Timeout:   time.Second * 30,
		Transport: cNetTransport,
	}
	for i := 0; i < runtime.GOMAXPROCS(-1); i++ {
		wg.Add(1)
		go func(id int) {
			// Constructing a client per goroutine.
			var dialer = &net.Dialer{
				Timeout: 10 * time.Second,
			}
			var netTransport = &http.Transport{
				DialContext:         dialer.DialContext,
				TLSHandshakeTimeout: 10 * time.Second,
				MaxIdleConns:        20,
				MaxIdleConnsPerHost: 10,
				IdleConnTimeout:     60 * time.Second,
			}
			var netClient = &http.Client{
				Timeout:   time.Second * 30,
				Transport: netTransport,
			}
			if *concurrent {
				// Using concurrent client instead.
				netClient = cNetClient
			}
			for {
				buf := make([]byte, 10000)
				req, err := http.NewRequest(http.MethodPost, *url+fmt.Sprintf("/%d", id), bytes.NewReader(buf))
				if err != nil {
					log.Fatal(err)
				}
				res, err := netClient.Do(req)
				if err != nil {
					fmt.Println("err", err)
					continue
				}
				// Ensuring connection reuse.
				io.Copy(ioutil.Discard, res.Body)
				res.Body.Close()

				if res.StatusCode != http.StatusOK {
					fmt.Println(res.Status)
				}
			}
		}(i)
	}
	wg.Wait()
}

Nginc config:

server {
        listen 443 ssl http2;
        location / {
                proxy_pass_request_headers on;
                proxy_pass http://localhost:8544;
        }

        server_name hostname;
        ssl_certificate     /home/ernado/.lego/certificates/hostname.crt;
        ssl_certificate_key /home/ernado/.lego/certificates/hostname.key;
}
$ nginx -v
nginx version: nginx/1.13.12

I was running client like that:

$ http2-test --client

What did you expect to see?

No errors in nginx log and client stdout.

What did you see instead?

In nginx error log:

2018/05/31 03:23:57 [info] 32265#32265: *40872 client prematurely closed stream: only 0 out of 10000 bytes of request body received while sending request to upstream, client: 127.0.0.1, server: HOST, request: "POST /1 HTTP/2.0", upstream: "http://127.0.0.1:8544/1", host: "HOST"

In client stdout:

400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request
400 Bad Request

If each spawned goroutine got a separate http client, there are no errors:

$ http2-test --client --concurrent=false

Also there are no errors in stdout if http2 is disabled

$ export GODEBUG=http2client=0
$ http2-test --client 

However, there are a lot of entries like this in nginx error log:

client 127.0.0.1 closed keepalive connection
client closed connection while waiting for request, client: 127.0.0.1, server: 0.0.0.0:443

But those entries disappear with --concurrent=false.

I'm not sure about the cause of that issue and will be happy if someone reproduce it too.

@ernado ernado changed the title x/net/http2: go client closes stream prematurely if used concurrently x/net/http2: http client closes stream prematurely if used concurrently May 31, 2018
@ernado

This comment has been minimized.

Copy link
Contributor Author

@ernado ernado commented May 31, 2018

cc: @bradfitz

@agnivade

This comment has been minimized.

Copy link
Contributor

@agnivade agnivade commented May 31, 2018

Duplicate of #25009. Please check that issue and close this if you think that is the same issue.

@ernado

This comment has been minimized.

Copy link
Contributor Author

@ernado ernado commented May 31, 2018

Yes, seems like a duplicate, thank you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.