Data race: App.WebSocket writes ctx.Context while serveWithGoroutine reads it (Test_WebSocket_Success fails under -race)
- Dominant language
- Go
- Stars
- 20.9k
- Forks
- 1.8k
- Avg merge
- 5d 18h
- Merged PRs (30d)
- 39
Description
**Describe the bug**
`App.WebSocket`'s handler writes `ctx.Context` while `serveWithGoroutine` concurrently reads
`c.Context` in its `select`, so every WebSocket request races the framework's own handler wrapper.
This is the websocket race deferred in 050596e8 ("The gRPC server ... and a pre-existing websocket
race are out of scope ... tracked separately"), which never got an issue. The gRPC half is #3929.
**The race**
Write, on the goroutine `serveWithGoroutine` spawned:
```go
// pkg/gofr/websocket.go:41
ctx.Context = context.WithValue(ctx, websocket.WSConnectionKey, conn)
```
Read, on the goroutine that spawned it:
```go
// pkg/gofr/handler.go:180
select {
case <-c.Context.Done():
```
`serveWithGoroutine` was written to avoid exactly this — its own comment says "shared variables, so
the goroutine never writes a memory location the main goroutine reads — `go test -race` stays
clean". That holds for the outcome value, which is passed over a channel, but not for `c.Context`:
the handler is handed the same `*gofr.Context` and is free to reassign the field, which
`App.WebSocket` does on every connection.
**Relationship to #3823**
Same class — an unsynchronised write to `c.Context` that another goroutine reads — but a different
write site and a different reader. #3823 is `Context.Trace()` racing user code in gRPC streaming
handlers; this one is framework code (`App.WebSocket`) racing framework code
(`serveWithGoroutine`), and it reproduces in this repo's own test suite with no user code involved.
Worth deciding together: a fix that makes `c.Context` safe to reassign (or removes the need to
reassign it) would close both. Filing separately rather than burying it in #3823's thread.
**To Reproduce**
```
go test ./pkg/gofr/ -race -count=1 -short -run 'Test_WebSocket_Success'
```
Reproduces on clean `development` (verified at `ceb50390`); `--- FAIL: Test_WebSocket_Success ...
race detected during execution of test`.
Note `go test ./pkg/gofr/websocket/... -race` is green — the race is in the handler wiring in
`pkg/gofr`, not in the websocket package itself.
**Expected behaviour**
A WebSocket handler storing the connection on its context does not race the request wrapper, and
`go test -race ./pkg/gofr/` is clean.
Contributor guide
Research direction
Start with pkg/gofr/websocket.go:41 and pkg/gofr/handler.go:180, then run go test ./pkg/gofr/ -race -count=1 -short -run 'Test_WebSocket_Success' to reproduce the race. Trace how the shared *gofr.Context is used by App.WebSocket and serveWithGoroutine, and compare the related context race in #3823. Done means the WebSocket test and go test -race ./pkg/gofr/ complete without a race.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100