go: net: Conn hangs with SetReadDeadline

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

$ go version
1.13

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="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/harsha/.cache/go-build"
GOENV="/home/harsha/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/harsha/mygo"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/harsha/.gimme/versions/go1.13.linux.amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/harsha/.gimme/versions/go1.13.linux.amd64/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/harsha/mygo/src/github.com/minio/minio/go.mod"
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-build110074343=/tmp/go-build -gno-record-gcc-switches"

What did you do?

~https://play.golang.org/p/Xpw5O5EsXQv~ https://play.golang.org/p/C-Jdtg6Wr9M

$ NO_TIMEOUT=0 go run /tmp/http.go

<?php
$homepage = file_get_contents('http://localhost:1234/');
echo $homepage;
?>

~ for i in $(seq 1 5); do /usr/bin/time -f "%E" php7.2 http.php 1>/dev/null; done
0:05.04
0:00.06
0:05.02
0:05.05
0:05.04

What did you expect to see?

No hangs with following code

$ NO_TIMEOUT=1 go run /tmp/http.go

<?php
$homepage = file_get_contents('http://localhost:1234/');
echo $homepage;
?>

~ for i in $(seq 1 5); do /usr/bin/time -f "%E" php7.2 http.php 1>/dev/null; done
0:00.03
0:00.01
0:00.01
0:00.01
0:00.01

What did you see instead?

Hangs up to the SetReadDeadline timeout

$ NO_TIMEOUT=0 go run /tmp/http.go

<?php
$homepage = file_get_contents('http://localhost:1234/');
echo $homepage;
?>

~ for i in $(seq 1 5); do /usr/bin/time -f "%E" php7.2 http.php 1>/dev/null; done
0:05.04
0:00.06
0:05.02
0:05.05
0:05.04

This is another variant of the same issue https://github.com/golang/go/issues/21133 and the work-around presented works still.

import (
	"net"
	"sync/atomic"
	"time"
)

// QuirkConn - similar to golang net.Conn struct, but contains a workaround of the
// following the go bug reported here https://github.com/golang/go/issues/21133.
// Once the bug will be fixed, we can remove this structure and replaces it with
// the standard net.Conn
type QuirkConn struct {
	net.Conn
	hadReadDeadlineInPast int32 // atomic
}

// SetReadDeadline - implements a workaround of SetReadDeadline go bug
func (q *QuirkConn) SetReadDeadline(t time.Time) error {
	inPast := int32(0)
	if t.Before(time.Now()) {
		inPast = 1
	}
	atomic.StoreInt32(&q.hadReadDeadlineInPast, inPast)
	return q.Conn.SetReadDeadline(t)
}

// canSetReadDeadline - returns if it is safe to set a new
// read deadline without triggering golang/go#21133 issue.
func (q *QuirkConn) canSetReadDeadline() bool {
	return atomic.LoadInt32(&q.hadReadDeadlineInPast) != 1
}

The question I have is why this workaround is needed at all. Is it not expected to SetReadDeadline() right before the read operation? but why does it work some times? It also looks like there may be some performance implications on doing something like this.

About this issue

Original URL
State: open
Created 5 years ago
Comments: 16 (9 by maintainers)

Commits related to this issue

Add a work-around to fix the SetReadDeadline() bug This commits fixes a bug introduced in af6c6a2b35ecd67300739f7ecb54fad60eeb4967. This workaround is needed to fix the upstream Go issue https://git... — committed to harshavardhana/minio by harshavardhana 5 years ago
Remove setting Deadlines as its not needed anymore This commits fixes a bug introduced in af6c6a2b35ecd67300739f7ecb54fad60eeb4967. Setting deadlines in Go results in arbitrary hangs as reported her... — committed to harshavardhana/minio by harshavardhana 5 years ago
Remove setting net.Conn Deadlines as its not needed anymore This commits fixes a bug introduced in af6c6a2b35ecd67300739f7ecb54fad60eeb4967. Setting deadlines in Go results in arbitrary hangs as rep... — committed to harshavardhana/minio by harshavardhana 5 years ago
Remove setting net.Conn Deadlines as its not needed anymore This commits fixes a bug introduced in af6c6a2b35ecd67300739f7ecb54fad60eeb4967. Setting deadlines in Go results in arbitrary hangs as rep... — committed to harshavardhana/minio by harshavardhana 5 years ago
Remove setting net.Conn Deadlines as its not needed anymore (#8269) This commit fixes a bug introduced in af6c6a2b35ecd67300739f7ecb54fad60eeb4967. Setting deadlines in Go results in arbitrary han... — committed to minio/minio by harshavardhana 5 years ago

Most upvoted comments

… without deadlines the code works fine. … either deadlines don’t work the way we want or we are using it wrong. …

When read deadline is not set in customizedConn.Read, current implementation of net/http uses very short implicit deadline and time-outs almost instantly. What you call “code works fine” is actually “problems are not given sufficient time to manifest”.

av86743 on Sep 21, 2019

@harshavardhana It’s documented in #21133 why your code doesn’t work, and what the workaround is. If the workaround has problems, you should open a new issue with code that demonstrates them.

Note curl is a better test client; not that many folks have PHP installed.

P.S. you don’t have to respond to every comment you get on an issue 😃

networkimprov on Sep 20, 2019

Second: An original example from #21133 with your setting of port :1234 is runnable from playground all right.

This is not a question and as such, it does not require your response. This is statement of a fact contrary to what you have earlier stated (I quote):

Playground doesn’t run HTTP service @av86743 its a nacl container 😃 any long-running go-routines such as HTTP service are rejected.

… This issue also talks about how to reproduce it using the command line using PHP code. … you need to use PHP to reproduce this issue not Go HTTP client. …

If you are not able to reproduce whatever it is without involving PHP, why do you claim that problem is on the go side and not on the PHP side?

av86743 on Sep 20, 2019

@av86743 because original was never addressed, what I wanted to shed light on is that the workaround provided was also not correct because.

It basically disabled SetReadDeadline() altogether and that exactly what shouldn’t happen. Setting deadlines repeatedly shouldn’t cause hangs in the first place and the workaround is also wrong because it basically just disabled the SetReadDeadline() code.

First: Why put your example in playground if it is not runnable from there?

This is an odd argument, the intention is to show code how to reproduce it. This issue also talks about how to reproduce it using the command line using PHP code. I don’t know what made you think that you can run in the playground and also run PHP code to reproduce this.

Second: An original example from #21133 with your setting of port :1234 is runnable from playground all right.

The original example added client code, which I didn’t want to add because you need to use PHP to reproduce this issue not Go HTTP client. Please follow what I pasted above on how to reproduce it.

harshavardhana on Sep 20, 2019