diff --git a/app/web_conn.go b/app/web_conn.go index 1a33e37be7..58fde26773 100644 --- a/app/web_conn.go +++ b/app/web_conn.go @@ -58,6 +58,7 @@ type WebConn struct { isWindows bool endWritePump chan struct{} pumpFinished chan struct{} + closeOnce sync.Once } // NewWebConn returns a new WebConn instance. @@ -94,15 +95,20 @@ func (a *App) NewWebConn(ws net.Conn, session model.Session, t i18n.TranslateFun } // Close closes the WebConn. +// It is made idempotent in nature by using a sync.Once +// to avoid a race condition that happens when an EventReadHup event +// and a connection close event happens at the same time. func (wc *WebConn) Close() { - wc.WebSocket.Close() - if !wc.isWindows { - // This triggers the pump exit. - // If the pump has already exited, this just becomes a noop. - close(wc.endWritePump) - } - // We wait for the pump to fully exit. - <-wc.pumpFinished + wc.closeOnce.Do(func() { + wc.WebSocket.Close() + if !wc.isWindows { + // This triggers the pump exit. + // If the pump has already exited, this just becomes a noop. + close(wc.endWritePump) + } + // We wait for the pump to fully exit. + <-wc.pumpFinished + }) } // GetSessionExpiresAt returns the time at which the session expires. diff --git a/app/web_hub_test.go b/app/web_hub_test.go index e7d0935cdf..94f69d503d 100644 --- a/app/web_hub_test.go +++ b/app/web_hub_test.go @@ -90,6 +90,20 @@ func TestHubStopWithMultipleConnections(t *testing.T) { defer wc3.Close() } +func TestWebConnDoubleClose(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + s := httptest.NewServer(dummyWebsocketHandler(t)) + defer s.Close() + + wc1 := registerDummyWebConn(t, th.App, s.Listener.Addr(), "userID") + wc1.Close() + require.NotPanics(t, func() { + wc1.Close() + }) +} + // TestHubStopRaceCondition verifies that attempts to use the hub after it has shutdown does not // block the caller indefinitely. func TestHubStopRaceCondition(t *testing.T) {