coder / coder/websocket

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

Abierto
#573 2 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Go
Estrellas
5.5k
Forks
372
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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?

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
go
Área
networking
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Necesita aclaración
Aptitud para principiantes
42/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.