Skip to content

Commit

Permalink
fix(client-ws-transport): Flush send queue in close() to avoid race (#…
Browse files Browse the repository at this point in the history
…9101)

Before this there were a race between `setTimeout` in `sendMessage` and `close`.
Even for a good citizen, that calls `unsubscribe` and awaits result there were no synchronization between `unsubscribe` adding message to `messageQueue` and restarting timer, and actually sending this message to WebSocket.
When `close()` is called close frame is sent immediately to WebSocket. If at that moment `messageQueue` is not empty, and timer is armed, it can wake up, and discover socket in CLOSING state: send side closed, recv side open.

For now - just flush everything queued, and send close frame after, so it would look like close was queued after.
More complete solution is to add synchronization between unsubscribe call and sending message, or even receiving ack for it.
  • Loading branch information
mcheshkov authored Jan 15, 2025
1 parent 80f10ce commit d9bc147
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions packages/cubejs-client-ws-transport/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ class WebSocketTransport implements ITransport<WebSocketTransportResult> {

public async close(): Promise<void> {
if (this.ws) {
// Flush send queue before sending close frame
this.ws.sendQueue();

this.ws.close();
}
}
Expand Down

0 comments on commit d9bc147

Please sign in to comment.