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

Data race on Conn.closeErr #436

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/websocket@v1.8.10/close.go:288 +0xd0
local-setup-websocket-api-1  |   nhooyr.io/websocket.(*Conn).setCloseErr()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/websocket@v1.8.10/close.go:282 +0x4c
local-setup-websocket-api-1  |   nhooyr.io/websocket.(*Conn).timeoutLoop()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/websocket@v1.8.10/conn.go:190 +0x224
local-setup-websocket-api-1  |   nhooyr.io/websocket.newConn.func2()
local-setup-websocket-api-1  |       /go/pkg/mod/nhooyr.io/websocket@v1.8.10/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/websocket@v1.8.10/close.go:117 +0x138
...
...

Apparently it's because when it's being set, it's protected by c.closeMu.

c.closeErr = fmt.Errorf("WebSocket closed: %w", err)

However it's not when it's being read.

return c.closeErr

@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 nhooyr#427
Closes nhooyr#429
Closes nhooyr#434
Closes nhooyr#436
Closes nhooyr#437
@nhooyr
Copy link
Owner

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 nhooyr#427
Closes nhooyr#429
Closes nhooyr#434
Closes nhooyr#436
Closes nhooyr#437
@nhooyr nhooyr added this to the v1.8.11 milestone Apr 5, 2024
@nhooyr
Copy link
Owner

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