Skip to content

Data race on Conn.closeErr #436

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

Closed
haha454 opened this issue Feb 22, 2024 · 3 comments
Closed

Data race on Conn.closeErr #436

haha454 opened this issue Feb 22, 2024 · 3 comments
Milestone

Comments

@haha454
Copy link

haha454 commented Feb 22, 2024

When I run my go program with race detector enabled, following logs were observed.

WARNING: DATA RACE
local-setup-websocket-api-1  | Write at 0x00c0002e31b0 by goroutine 129:
local-setup-websocket-api-1  |   nhooyr.io/websocket.(*Conn).setCloseErrLocked()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/[email protected]/close.go:288 +0xd0
local-setup-websocket-api-1  |   nhooyr.io/websocket.(*Conn).setCloseErr()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/[email protected]/close.go:282 +0x4c
local-setup-websocket-api-1  |   nhooyr.io/websocket.(*Conn).timeoutLoop()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/[email protected]/conn.go:190 +0x224
local-setup-websocket-api-1  |   nhooyr.io/websocket.newConn.func2()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/[email protected]/conn.go:137 +0x80
local-setup-websocket-api-1  |
local-setup-websocket-api-1  | Previous read at 0x00c0002e31b0 by goroutine 165:
local-setup-websocket-api-1  |   nhooyr.io/websocket.(*Conn).CloseNow()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/[email protected]/close.go:117 +0x138
...
...

Apparently it's because when it's being set, it's protected by c.closeMu.
https://github.com/nhooyr/websocket/blob/e3a2d32f704fb06c439e56d2a85334de04b50d32/close.go#L288

However it's not when it's being read.
https://github.com/nhooyr/websocket/blob/e3a2d32f704fb06c439e56d2a85334de04b50d32/close.go#L117

@haha454
Copy link
Author

haha454 commented Feb 22, 2024

I realise there is a PR #427 for it.

nhooyr added a commit to alixander/websocket that referenced this issue Apr 5, 2024
Far simpler now. Sorry this took a while.

Closes coder#427
Closes coder#429
Closes coder#434
Closes coder#436
Closes coder#437
@nhooyr
Copy link
Contributor

nhooyr commented Apr 5, 2024

Thanks for reporting. I've fixed this in #427. Please review and let me know if you see anything else.

nhooyr added a commit to alixander/websocket that referenced this issue Apr 5, 2024
Far simpler now. Sorry this took a while.

Closes coder#427
Closes coder#429
Closes coder#434
Closes coder#436
Closes coder#437
@nhooyr nhooyr added this to the v1.8.11 milestone Apr 5, 2024
@nhooyr
Copy link
Contributor

nhooyr commented Apr 7, 2024

Sorry for the delay. Fixed by #427.

@nhooyr nhooyr closed this as completed Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants