Skip to content

websocket.Conn.CloseNow() panics in WaitGroup.Wait #437

Closed

Description

@tomasji

nhooyr.io/websocket v1.8.10

When CloseNow() is called by 2 goroutines, there is a race condition:

panic: sync: WaitGroup is reused before previous Wait has returned

goroutine 105321 [running]:
sync.(*WaitGroup).Wait(0xe33400?)
	/src/sync/waitgroup.go:118 +0x74
nhooyr.io/websocket.(*Conn).CloseNow(0xc02d1e5860)
	/.go/pkg/mod/nhooyr.io/websocket@v1.8.10/close.go:113 +0xcb

Original func

func (c *Conn) CloseNow() (err error) {
	defer c.wg.Wait()
	defer errd.Wrap(&err, "failed to close WebSocket")

	if c.isClosed() {
		return net.ErrClosed
	}
...
}

Possible fix: move defer calls below isClosed()

func (c *Conn) CloseNow() (err error) {
	if c.isClosed() {
		return net.ErrClosed
	}
	defer c.wg.Wait()
	defer errd.Wrap(&err, "failed to close WebSocket")
...
}

Activity

  1. added a commit that references this issue on Apr 5, 2024
    231f9ae
  2. nhooyr commented on Apr 5, 2024

    @nhooyr
    Contributor

    Thanks for reporting. I've fixed this in #427. Please review and let me know if you see anything else.

  3. added a commit that references this issue on Apr 5, 2024
    db18a31
  4. added this to the v1.8.11 milestone on Apr 5, 2024
  5. nhooyr commented on Apr 7, 2024

    @nhooyr
    Contributor

    Sorry for the delay. Fixed by #427.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions