coder / coder/websocket

mu.lock: after acquiring m.ch, why does the re-check only look at closed and not ctx?

Open
#573 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
5.5k
Forks
372
PR merge metrics
No merged PRs in 30d

Description

Hi, I was looking at [mu.lock](https://github.com/coder/websocket/blob/master/conn.go#L286) and had a question about this part:

```go
func (m *mu) lock(ctx context.Context) error {
select {
case <-m.c.closed:
return net.ErrClosed
case <-ctx.Done():
return fmt.Errorf("failed to acquire lock: %w", ctx.Err())
case m.ch <- struct{}{}:
// To make sure the connection is certainly alive.
// As it's possible the send on m.ch was selected
// over the receive on closed.
select {
case <-m.c.closed:
// Make sure to release.
m.unlock()
return net.ErrClosed
default:
}
return nil
}
}
```

After acquiring the lock, `closed` is checked again in case both branches were ready.

Why isn't `ctx.Done()` checked here as well?

Could `ctx` be canceled right after `m.ch` is selected, causing `lock` to return `nil` while holding the lock with an already-canceled context?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with conn.go around mu.lock at line 286 and trace how its context and closed channels are used by callers. Compare the cancellation and lock-acquisition behavior, then determine whether the reported case requires a code or documentation change; done means the issue's question is answered with a reproducible rationale.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.