Skip to content

Commit

Permalink
fix: deadlock when resetting connection (#798)
Browse files Browse the repository at this point in the history
  • Loading branch information
acomley-stripe committed Dec 8, 2021
1 parent 0aeb810 commit b424ed8
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 2 deletions.
14 changes: 12 additions & 2 deletions pkg/websocket/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (c *Client) Close(closeCode int, text string) {
err := c.conn.WriteControl(ws.CloseMessage, message, time.Now().Add(c.cfg.WriteWait))
if err != nil {
c.cfg.Log.WithFields(log.Fields{
"prefix": "websocket.Client.Run",
"prefix": "websocket.Client.Close",
"error": err,
}).Debug("Error while trying to send close frame")
}
Expand Down Expand Up @@ -437,8 +437,18 @@ func (c *Client) writePump() {
if ws.IsUnexpectedCloseError(err, ws.CloseNormalClosure) {
c.cfg.Log.Error("write error: ", err)
}
c.notifyClose <- err

// writing to notifyClose during a reset will cause a deadlock
select {
case c.notifyClose <- err:
c.cfg.Log.WithFields(log.Fields{
"prefix": "websocket.Client.writePump",
}).Debug("Failed to send ping; closing connection")
case <-c.stopWritePump:
c.cfg.Log.WithFields(log.Fields{
"prefix": "websocket.Client.writePump",
}).Debug("Failed to send ping; connection is resetting")
}
return
}
case <-c.stopWritePump:
Expand Down
54 changes: 54 additions & 0 deletions pkg/websocket/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

ws "github.com/gorilla/websocket"
// log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -174,3 +175,56 @@ func TestClientExpiredError(t *testing.T) {
require.FailNow(t, "Timed out waiting for response from test server")
}
}

/* func TestClientWebhookReconnect(t *testing.T) {
log.SetLevel(log.DebugLevel)
wg := &sync.WaitGroup{}
wg.Add(20)
upgrader := ws.Upgrader{}
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
c, err := upgrader.Upgrade(w, r, nil)
require.NoError(t, err)
defer c.Close()
swg := &sync.WaitGroup{}
swg.Add(1)
go func() {
for {
if _, _, err := c.ReadMessage(); err != nil {
swg.Done()
return
}
}
}()
swg.Wait()
wg.Done()
}))
defer ts.Close()
url := "ws" + strings.TrimPrefix(ts.URL, "http")
rcvMsgChan := make(chan WebhookEvent)
client := NewClient(
url,
"websocket-random-id",
"webhook-payloads",
&Config{
EventHandler: EventHandlerFunc(func(msg IncomingMessage) {
rcvMsgChan <- *msg.WebhookEvent
}),
Log: log.StandardLogger(),
ReconnectInterval: 10 * time.Second,
},
)
go client.Run(context.Background())
defer client.Stop()
wg.Wait()
} */

0 comments on commit b424ed8

Please sign in to comment.