search

pion / stun

A Go implementation of STUN
https://pion.ly/
MIT License
616 stars 94 forks source link

Close hangs app when WithNoConnClose #80

Open adsko opened 3 years ago

adsko commented 3 years ago

When stun connection is created from existing connection, and WithNoConnClose is provided, Close hangs app.

hashok commented 3 years ago

I have the same issue. Debugging showed that Close waits for readUntilClosed go-routing to exit, but that go-routine is blocked on ReadFrom call here https://github.com/pion/stun/blob/0f7f72b905d54eaf8e477af78c6a3d42f54ea217/client.go#L327

Workaround is to call SetReadDeadline(time.Now()) before calling Close for the stun.Client. It causes ReadFrom to immediately exit.

adsko commented 3 years ago

@hashok Calling SetReadDeadline(time.Now()) closes the connection.

hashok commented 3 years ago

@adsko It shouldn't do that. Make sure you call SetReadDeadline(time.Time{}) on the underlying connection after stun connection is closed to restore the original behavior of the underlying connection.

stv0g commented 1 year ago

How do we fix this issue?

Setting the read deadline in Close() is not feasible since the connection only implements the following interface:

// Connection wraps Reader, Writer and Closer interfaces.
type Connection interface {
    io.Reader
    io.Writer
    io.Closer
}

We could call c.wg.Done() manually to unblock the c.wg.Wait() in Close(). However, that would leave us with a leaked Goroutine and might cause some panic if the connection still ends up receiving data while the client has been closed?