coder / coder/websocket

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

Offen
#573 2 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Go
Sterne
5.5k
Forks
372
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

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?

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Rechercherichtung

Beginne in conn.go bei mu.lock in Zeile 286 und verfolge, wie sein Kontext und geschlossene Channels von den Aufrufern verwendet werden. Vergleiche das Verhalten bei der Abbruchbehandlung und beim Erwerben des Locks und bestimme anschließend, ob der gemeldete Fall eine Code- oder Dokumentationsänderung erfordert; abgeschlossen bedeutet, dass die Frage des Issues mit einer reproduzierbaren Begründung beantwortet ist.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
go
Bereich
networking
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Aktiv
Klarheit
Muss geklärt werden
Anfängerfreundlichkeit
42/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.