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: proxied connection can be spuriously closed after dialConn as of CL 210286 #36070

Closed
bcmills opened this issue Dec 10, 2019 · 9 comments
Closed

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Dec 10, 2019

A timeout in net/http.TestTransportProxy seems to have started around CL 210286:

panic: test timed out after 3m0s

goroutine 2138 [running]:
testing.(*M).startAlarm.func1()
	/workdir/go/src/testing/testing.go:1422 +0x9
created by time.goFunc
	/workdir/go/src/time/sleep.go:168 +0x5

[…]

goroutine 2125 [chan receive]:
net/http_test.TestTransportProxy.func1(0xb08360)
	/workdir/go/src/net/http/transport_test.go:1424 +0x5a
testing.tRunner(0xb08360, 0xa3af60)
	/workdir/go/src/testing/testing.go:954 +0xc
created by testing.(*T).Run
	/workdir/go/src/testing/testing.go:1005 +0x30

[…]

goroutine 2107 [chan receive, 2 minutes]:
testing.(*T).Run(0xb08360, 0xa62860, 0x20, 0xa3af60, 0x2)
	/workdir/go/src/testing/testing.go:1006 +0x31
net/http_test.TestTransportProxy(0x875d40)
	/workdir/go/src/net/http/transport_test.go:1324 +0x6
testing.tRunner(0x875d40, 0x13aca0)
	/workdir/go/src/testing/testing.go:954 +0xc
created by testing.(*T).Run
	/workdir/go/src/testing/testing.go:1005 +0x30
FAIL	net/http	184.130s

2019-12-09T21:50:59-3a3093d/js-wasm
2019-12-09T20:19:32-7d24e95/js-wasm
2019-12-09T16:41:15-a3a630b/js-wasm

CC @bradfitz @ianlancetaylor

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Dec 10, 2019

That CL was the fix for #28012.

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 10, 2019

It passed the trybots at the time.

Does a js-wasm net.Conn not reliably interrupt a blocked Read on Close or something?

I'm fine just skipping that test on js-wasm for now.

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Dec 10, 2019

I don't know the underlying cause of the failure. The failure rate seems to be around 50%, which would explain the passing TryBot.

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Dec 10, 2019

(CC @neelance @cherrymui for the question about the js-wasm net implementation.)

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 10, 2019

The new code in CL 210286 adds a 1 minute timeout reading the CONNECT response if no context deadline was otherwise present.

It's possible the net/http tests are just so slow on wasm that it's straddling that 1 minute timeout already (~50% of the time?) with all the parallel tests. But, uh, TestTransportProxy isn't t.Parallel.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 10, 2019

Change https://golang.org/cl/210737 mentions this issue: net/http: set a context deadline in HEAD request in test

@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Dec 10, 2019

@bcmills bcmills changed the title net/http: TestTransportProxy timing out on js-wasm builder after CL 210286 net/http: proxied connection can be spuriously closed after DialConn as of CL 210286 Dec 10, 2019
@bcmills bcmills changed the title net/http: proxied connection can be spuriously closed after DialConn as of CL 210286 net/http: proxied connection can be spuriously closed after dialConn as of CL 210286 Dec 10, 2019
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 10, 2019

Whoops, thanks!

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 10, 2019

Change https://golang.org/cl/210738 mentions this issue: net/http: fix timeout race in Transport proxy CONNECT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.