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.

Sign up for GitHub

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

Jump to bottom

drain buffer correctly in httpupgrade #3428

Merged
merged 10 commits into from
Jun 6, 2024
Merged

drain buffer correctly in httpupgrade #3428

merged 10 commits into from
Jun 6, 2024

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Jun 6, 2024

it seems the recently added httupgrade testsuite is causing timeouts on master

i have no evidence this is the real issue, but it feels to me that the server could accidentally over-read, and then the encapsulated connection will block forever trying to read data

let's test it in CI a couple of times, i don't have a way to reproduce the issue

@mmmray
drain buffer correctly in httpupgrade
067d069 
it seems the recently added httupgrade testsuite is causing timeouts on master

i have no evidence this is the real issue, but it feels to me that the
server could accidentally over-read, and then the encapsulated
connection will block forever trying to read data

let's test it in CI a couple of times, i don't have a way to reproduce
the issue
@mmmray mmmray mentioned this pull request Jun 6, 2024
Merged
@mmmray
Copy link
Collaborator Author

mmmray commented Jun 6, 2024

the fix doesn't make any sense, the buffer will read even more data from the connection. fixing...

@mmmray mmmray marked this pull request as ready for review June 6, 2024 02:12
@mmmray
Copy link
Collaborator Author

mmmray commented Jun 6, 2024 •
edited
Loading

httpupgrade failure, from main branch 
2024-06-05T18:43:05.8378247Z panic: test timed out after 1h0m0s
2024-06-05T18:43:05.8378811Z running tests:
2024-06-05T18:43:05.8379389Z 	Test_listenHTTPUpgradeAndDial (1h0m0s)
2024-06-05T18:43:05.8379858Z 
2024-06-05T18:43:05.8380047Z goroutine 34 [running]:
2024-06-05T18:43:05.8380707Z testing.(*M).startAlarm.func1()
2024-06-05T18:43:05.8381902Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:2366 +0x385
2024-06-05T18:43:05.8383047Z created by time.goFunc
2024-06-05T18:43:05.8383989Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/time/sleep.go:177 +0x2d
2024-06-05T18:43:05.8384664Z 
2024-06-05T18:43:05.8384879Z goroutine 1 [chan receive]:
2024-06-05T18:43:05.8385358Z testing.(*T).Run(0xc00013a4e0, {0xe4c2c6?, 0xc0301ad6d4?}, 0xe7abb8)
2024-06-05T18:43:05.8386026Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:1750 +0x3ab
2024-06-05T18:43:05.8386594Z testing.runTests.func1(0xc00013a4e0)
2024-06-05T18:43:05.8387171Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:2161 +0x37
2024-06-05T18:43:05.8387728Z testing.tRunner(0xc00013a4e0, 0xc00007fc70)
2024-06-05T18:43:05.8388529Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:1689 +0xfb
2024-06-05T18:43:05.8389944Z testing.runTests(0xc0000082e8, {0x124a620, 0x3, 0x3}, {0x1?, 0x9ae1b3?, 0x1253c40?})
2024-06-05T18:43:05.8392787Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:2159 +0x445
2024-06-05T18:43:05.8393857Z testing.(*M).Run(0xc000160140)
2024-06-05T18:43:05.8394998Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:2027 +0x68b
2024-06-05T18:43:05.8395549Z main.main()
2024-06-05T18:43:05.8395840Z 	_testmain.go:53 +0x16c
2024-06-05T18:43:05.8396058Z 
2024-06-05T18:43:05.8396175Z goroutine 6 [IO wait]:
2024-06-05T18:43:05.8396573Z internal/poll.runtime_pollWait(0x21ac6feb1a8, 0x72)
2024-06-05T18:43:05.8397206Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/runtime/netpoll.go:345 +0x85
2024-06-05T18:43:05.8397786Z internal/poll.(*pollDesc).wait(0x1?, 0x0?, 0x0)
2024-06-05T18:43:05.8398445Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_poll_runtime.go:84 +0x27
2024-06-05T18:43:05.8399065Z internal/poll.execIO(0xc0000ca2a0, 0xe7b278)
2024-06-05T18:43:05.8400534Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:175 +0xe6
2024-06-05T18:43:05.8401889Z internal/poll.(*FD).Read(0xc0000ca288, {0x12dd4c0, 0x0, 0x0})
2024-06-05T18:43:05.8402618Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:436 +0x2b1
2024-06-05T18:43:05.8403307Z net.(*netFD).Read(0xc0000ca288, {0x12dd4c0?, 0x1?, 0xc000086240?})
2024-06-05T18:43:05.8403951Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/fd_posix.go:55 +0x25
2024-06-05T18:43:05.8404549Z net.(*conn).Read(0xc00008c020, {0x12dd4c0?, 0xe4005f?, 0xa?})
2024-06-05T18:43:05.8405155Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/net.go:179 +0x45
2024-06-05T18:43:05.8407307Z github.com/xtls/xray-core/transport/internet/httpupgrade.(*ConnRF).Read(0xc00009e2c0?, {0x12dd4c0?, 0x0?, 0x0?})
2024-06-05T18:43:05.8408338Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/dialer.go:38 +0x83
2024-06-05T18:43:05.8409527Z github.com/xtls/xray-core/transport/internet/httpupgrade.dialhttpUpgrade({0xf02c38, 0x12dd4c0}, {{0xf02c70, 0xc00003a5f0}, 0xc57a, 0x2}, 0xc00003fea8)
2024-06-05T18:43:05.8410663Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/dialer.go:94 +0xae7
2024-06-05T18:43:05.8411783Z github.com/xtls/xray-core/transport/internet/httpupgrade.Dial({0xf02c38, 0x12dd4c0}, {{0xf02c70, 0xc00003a5f0}, 0xc57a, 0x2}, 0xc000069ea8)
2024-06-05T18:43:05.8413778Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/dialer.go:106 +0x189
2024-06-05T18:43:05.8415916Z github.com/xtls/xray-core/transport/internet/httpupgrade_test.Test_listenHTTPUpgradeAndDial(0xc00013a680)
2024-06-05T18:43:05.8417045Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/httpupgrade_test.go:46 +0x1a5
2024-06-05T18:43:05.8417658Z testing.tRunner(0xc00013a680, 0xe7abb8)
2024-06-05T18:43:05.8418241Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:1689 +0xfb
2024-06-05T18:43:05.8418818Z created by testing.(*T).Run in goroutine 1
2024-06-05T18:43:05.8419418Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/testing/testing.go:1742 +0x390
2024-06-05T18:43:05.8419835Z 
2024-06-05T18:43:05.8419952Z goroutine 8 [IO wait]:
2024-06-05T18:43:05.8420336Z internal/poll.runtime_pollWait(0x21ac6feb2a0, 0x72)
2024-06-05T18:43:05.8421374Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/runtime/netpoll.go:345 +0x85
2024-06-05T18:43:05.8421996Z internal/poll.(*pollDesc).wait(0x8dc2d6?, 0x12deae0?, 0x0)
2024-06-05T18:43:05.8422707Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_poll_runtime.go:84 +0x27
2024-06-05T18:43:05.8423345Z internal/poll.execIO(0xc0000776a0, 0xc000065d28)
2024-06-05T18:43:05.8423987Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:175 +0xe6
2024-06-05T18:43:05.8424836Z internal/poll.(*FD).acceptOne(0xc000077688, 0x24c, {0xc0000d6000?, 0xc0000d0201?, 0xc000065f70?}, 0xd556cf?)
2024-06-05T18:43:05.8425683Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:944 +0x67
2024-06-05T18:43:05.8426641Z internal/poll.(*FD).Accept(0xc000077688, 0xc000065f08)
2024-06-05T18:43:05.8427412Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:978 +0x1bc
2024-06-05T18:43:05.8429012Z net.(*netFD).accept(0xc000077688)
2024-06-05T18:43:05.8429563Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/fd_windows.go:178 +0x54
2024-06-05T18:43:05.8430113Z net.(*TCPListener).accept(0xc000044ec0)
2024-06-05T18:43:05.8430695Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/tcpsock_posix.go:159 +0x1e
2024-06-05T18:43:05.8431224Z net.(*TCPListener).Accept(0xc000044ec0)
2024-06-05T18:43:05.8431883Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/tcpsock.go:327 +0x30
2024-06-05T18:43:05.8433787Z github.com/xtls/xray-core/transport/internet/httpupgrade.(*server).keepAccepting(0xc000044ee0)
2024-06-05T18:43:05.8435736Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/hub.go:87 +0x2f
2024-06-05T18:43:05.8437653Z created by github.com/xtls/xray-core/transport/internet/httpupgrade.ListenHTTPUpgrade in goroutine 6
2024-06-05T18:43:05.8439913Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/hub.go:145 +0x9c9
2024-06-05T18:43:05.8440750Z 
2024-06-05T18:43:05.8440885Z goroutine 20 [IO wait]:
2024-06-05T18:43:05.8441303Z internal/poll.runtime_pollWait(0x21ac6feb0b0, 0x72)
2024-06-05T18:43:05.8441929Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/runtime/netpoll.go:345 +0x85
2024-06-05T18:43:05.8442546Z internal/poll.(*pollDesc).wait(0x1?, 0xc00016bd48?, 0x0)
2024-06-05T18:43:05.8443228Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_poll_runtime.go:84 +0x27
2024-06-05T18:43:05.8443853Z internal/poll.execIO(0xc0000ca520, 0xe7b278)
2024-06-05T18:43:05.8444478Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:175 +0xe6
2024-06-05T18:43:05.8445165Z internal/poll.(*FD).Read(0xc0000ca508, {0xc0000d8000, 0x400, 0x400})
2024-06-05T18:43:05.8446376Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/internal/poll/fd_windows.go:436 +0x2b1
2024-06-05T18:43:05.8447061Z net.(*netFD).Read(0xc0000ca508, {0xc0000d8000?, 0x2?, 0x2?})
2024-06-05T18:43:05.8447674Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/fd_posix.go:55 +0x25
2024-06-05T18:43:05.8448258Z net.(*conn).Read(0xc00008c018, {0xc0000d8000?, 0x0?, 0x0?})
2024-06-05T18:43:05.8448837Z 	C:/hostedtoolcache/windows/go/1.22.4/x64/src/net/net.go:179 +0x45
2024-06-05T18:43:05.8449913Z github.com/xtls/xray-core/transport/internet/httpupgrade_test.Test_listenHTTPUpgradeAndDial.func1.1({0xf05838, 0xc00009e2e0})
2024-06-05T18:43:05.8450997Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/httpupgrade_test.go:31 +0x85
2024-06-05T18:43:05.8452672Z created by github.com/xtls/xray-core/transport/internet/httpupgrade_test.Test_listenHTTPUpgradeAndDial.func1 in goroutine 8
2024-06-05T18:43:05.8453783Z 	D:/a/Xray-core/Xray-core/transport/internet/httpupgrade/httpupgrade_test.go:27 +0x69
2024-06-05T18:43:05.8454604Z FAIL	github.com/xtls/xray-core/transport/internet/httpupgrade	3600.051s

another kcp test failure in this branch:

2024-06-06T02:12:46.0767208Z panic: unexpected EOF
2024-06-06T02:12:46.0767216Z 
2024-06-06T02:12:46.0767325Z goroutine 25 [running]:
2024-06-06T02:12:46.0767515Z github.com/xtls/xray-core/common.Must(...)
2024-06-06T02:12:46.0767793Z 	/home/runner/work/Xray-core/Xray-core/common/common.go:23
2024-06-06T02:12:46.0767973Z github.com/xtls/xray-core/common.Must2(...)
2024-06-06T02:12:46.0768237Z 	/home/runner/work/Xray-core/Xray-core/common/common.go:29
2024-06-06T02:12:46.0768617Z github.com/xtls/xray-core/transport/internet/kcp_test.TestDialAndListen.func2()
2024-06-06T02:12:46.0769028Z 	/home/runner/work/Xray-core/Xray-core/transport/internet/kcp/kcp_test.go:62 +0x42d
2024-06-06T02:12:46.0769186Z golang.org/x/sync/errgroup.(*Group).Go.func1()
2024-06-06T02:12:46.0769517Z 	/home/runner/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:78 +0x56
2024-06-06T02:12:46.0769766Z created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 22
2024-06-06T02:12:46.0770086Z 	/home/runner/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:75 +0x96
2024-06-06T02:12:46.0770355Z FAIL	github.com/xtls/xray-core/transport/internet/kcp	1.448s

@yuhan6665 yuhan6665 merged commit f8ec93d into XTLS:main Jun 6, 2024
34 checks passed
@yuhan6665
Copy link
Member

Looks good to me! The KCP issue is long-standing. I think UDP traffic test has some little chance of failing.

@mmmray mmmray deleted the bufio-httpupgrade branch June 6, 2024 09:44
@tqangxl tqangxl mentioned this pull request Jul 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mmmray @yuhan6665