go-socket.io
go-socket.io copied to clipboard
Fix race condition.
Describe the bug Race condition, see https://github.com/googollee/go-socket.io/runs/2513183135?check_suite_focus=true.
==================
WARNING: DATA RACE
Write at 0x00c00008b2a1 by goroutine 45:
testing.(*common).FailNow()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:740 +0x4f
testing.(*T).FailNow()
<autogenerated>:1 +0x44
github.com/stretchr/testify/require.Nil()
/home/runner/go/pkg/mod/github.com/stretchr/[email protected]/require/require.go:955 +0x104
github.com/stretchr/testify/require.(*Assertions).Nil()
/home/runner/go/pkg/mod/github.com/stretchr/[email protected]/require/require_forward.go:748 +0xd0
github.com/googollee/go-socket.io/engineio.TestEngineUpgrade.func1()
/home/runner/work/go-socket.io/go-socket.io/engineio/engine_test.go:197 +0x204
Previous write at 0x00c00008b2a1 by goroutine 43:
testing.(*common).FailNow()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:740 +0x4f
testing.(*T).FailNow()
<autogenerated>:1 +0x44
github.com/stretchr/testify/require.Nil()
/home/runner/go/pkg/mod/github.com/stretchr/[email protected]/require/require.go:955 +0x104
github.com/stretchr/testify/require.(*Assertions).Nil()
/home/runner/go/pkg/mod/github.com/stretchr/[email protected]/require/require_forward.go:748 +0xd0
github.com/googollee/go-socket.io/engineio.TestEngineUpgrade()
/home/runner/work/go-socket.io/go-socket.io/engineio/engine_test.go:282 +0x1008
testing.tRunner()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1193 +0x202
Goroutine 45 (running) created at:
github.com/googollee/go-socket.io/engineio.TestEngineUpgrade()
/home/runner/work/go-socket.io/go-socket.io/engineio/engine_test.go:186 +0x22c
testing.tRunner()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1193 +0x202
Goroutine 43 (running) created at:
testing.(*T).Run()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1238 +0x5d7
testing.runTests.func1()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1511 +0xa6
testing.tRunner()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1193 +0x202
testing.runTests()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1509 +0x612
testing.(*M).Run()
/opt/hostedtoolcache/go/1.16.3/x64/src/testing/testing.go:1417 +0x3b3
main.main()
_testmain.go:105 +0x356
==================
2021/05/05 21:27:06 httptest.Server blocked in Close after 5 seconds, waiting for connections:
*net.TCPConn 0xc000088108 127.0.0.1:57546 in state active
--- FAIL: TestEngineUpgrade (60.03s)
engine_test.go:282:
Error Trace: engine_test.go:282
Error: Expected nil, but got: &websocket.CloseError{Code:1006, Text:"unexpected EOF"}
Test: TestEngineUpgrade
engine_test.go:197:
Error Trace: engine_test.go:197
asm_amd64.s:1371
Error: Expected nil, but got: &errors.errorString{s:"EOF"}
Test: TestEngineUpgrade
testing.go:1092: race detected during execution of test
FAIL
To Reproduce
make test
. As it's a race condition, it's hard to reproduce it every time.
Expected behavior No race condition.
Environment (please complete the following information):
- Go version: 1.16
- Server version: 1.4
- Client version: 1.4
It is so strange. Into golang version 1.16 does not detected but into 1.15 was detected race. Is broken race condition detector into new go-version?
It could be 1.16 changed the scheduler and make it not so easy to trigger. I always feel that the upgrading process of old code is not a good design. That's why I hope to do a new design directly.
That's why I hope to do a new design directly.
That's good idea
I think maybe add old version 1.13 and 1.14 into github action. What do you think about it?
Just do it.
Hey, Googol! Is it actually? sorry, i don't understand. It is so strange behaviour
I believe it's very hard to fix with the current code. As I said in https://github.com/googollee/go-socket.io/issues/456#issuecomment-837243206, I'd like to design a new way to handle upgrade properly.