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

net/http: ReadHeaderTimeout didn't work proper with KeepAlive enabled #47007

Closed
molchalin opened this issue Jul 1, 2021 · 7 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@molchalin
Copy link

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

$ go version
$ go version go1.16.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/a.eremeev/Library/Caches/go-build"
GOENV="/Users/a.eremeev/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/a.eremeev/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/a.eremeev/go"
GOPRIVATE="stash.mail.ru/scm,gitlab.corp.mail.ru"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/a.eremeev/brew/Cellar/go/1.16.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/a.eremeev/brew/Cellar/go/1.16.5/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0k/19q3g2wx6zlc98ftvxnpq5b80000gp/T/go-build1670355217=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/8Za_9D3QqXC

I've sent http request with Keep Alive enabled(IdleConnTimeout = 90s) to server with ReadHeaderTimeout = 3s.

What did you expect to see?

no connection close

What did you see instead?

connection closed after 3s from last request

@fraenkel
Copy link
Contributor

fraenkel commented Jul 1, 2021

@molchalin Where did you set the IdleTimeout?
The code you show is working as expected.

@molchalin
Copy link
Author

@molchalin Where did you set the IdleTimeout?
The code you show is working as expected.

IdleTimeout has default value, so i suppose that server will not close connection

@fraenkel
Copy link
Contributor

fraenkel commented Jul 2, 2021

It has a default value of 0 which means no timeout.
At the end of the request, given no idle timeout, it will attempt to read the header of the next request which is why you hit the read header timeout.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 2, 2021
@dmitshur dmitshur added this to the Backlog milestone Jul 2, 2021
@dmitshur dmitshur added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 2, 2021
@rkuska
Copy link
Contributor

rkuska commented Jul 27, 2021

I've encountered the same issue with ReadHeaderTimeout. Setting up the IdleTimeout fixed this issue for me. But it is strange when ReadHeaderTimeout is not specified the IdleTimeout is not needed and connections are kept around without a problem. Maybe the documentation could be improved for *Timeout settings? To explicitly reference this somehow?

@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@AusIV
Copy link

AusIV commented Dec 6, 2021

Is it possible to reopen an issue here?

I believe the documentation needs to be clarified that if IdleTimeout and ReadTimeout are not set, it commences reading the next header, and thus will close the connection after ReadHeaderTimeout. The way it reads now, it appears that if no IdleTimeout or ReadTimeout is set, the connection will be kept open until the remote side closes the connection, which is not true if ReadHeaderTimeout is set.

This is the relevant section of documentation: https://pkg.go.dev/net/http#Server

@FilipVozar
Copy link

I found the same thing as @AusIV. I also found that this was fixed already in #20383, but then the fix was reverted in #32053.

example:

package main

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

func main() {
	t0 := time.Now()
	l, err := net.Listen("tcp", "127.0.0.1:0")
	if err != nil {
		fmt.Println("listen", err)
		return
	}
	fmt.Println("listening on", l.Addr())

	go func() {
		r, err := http.Get(fmt.Sprintf("http://%s/", l.Addr()))
		if err != nil {
			fmt.Println("request error", err)
			return
		}
		_, _ = io.Copy(io.Discard, r.Body)
	}()

	shutdown := make(chan struct{})
	srv := http.Server{
		ReadHeaderTimeout: 2 * time.Second,
		ConnState: func(conn net.Conn, state http.ConnState) {
			fmt.Println(time.Since(t0), conn.RemoteAddr(), "ConnState", state)
			if state == http.StateClosed {
				close(shutdown)
			}
		},
	}

	go func() {
		<-shutdown
		srv.Close()
	}()
	if err := srv.Serve(l); err != http.ErrServerClosed {
		fmt.Println("serve", err)
	}
}

example output:

listening on 127.0.0.1:50722
1.042992ms 127.0.0.1:50723 ConnState new
1.416261ms 127.0.0.1:50723 ConnState active
1.722343ms 127.0.0.1:50723 ConnState idle
2.002489271s 127.0.0.1:50723 ConnState closed

Note the connection was closed 2 seconds after becoming idle (the value of ReadHeaderTimeout). According to IdleTimeout docs, this behaviour is unexpected - the connection should not be closed, because both IdleTimeout and ReadTimeout are zero.

	// IdleTimeout is the maximum amount of time to wait for the
	// next request when keep-alives are enabled. If IdleTimeout
	// is zero, the value of ReadTimeout is used. If both are
	// zero, there is no timeout.
	IdleTimeout time.Duration

If we removed ReadHeaderTimeout, the server would not close the connection (in this particular example, the connection would be closed after 90 seconds by the http.DefaultTransport, as expected).

@golang golang locked and limited conversation to collaborators Dec 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

7 participants