gofr-dev / gofr-dev/gofr

Data race: App.WebSocket writes ctx.Context while serveWithGoroutine reads it (Test_WebSocket_Success fails under -race)

Open
#3,930 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.