Skip to content

Commit

Permalink
Don't drop hibernatable ws if pumping
Browse files Browse the repository at this point in the history
We had a bug where we can register two pump() promises because the
second pump() starts before the promise continuation of the first
completes. This meant that when the first pump() continuation started
running, it could cancel the second pump() task, preventing outgoing
messages from being sent.
  • Loading branch information
MellowYarker committed Jan 15, 2024
1 parent a38f1f4 commit a0a979b
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/workerd/api/web-socket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,18 @@ void WebSocket::ensurePumping(jsg::Lock& js) {
reportError(js, KJ_EXCEPTION(DISCONNECTED, "WebSocket peer disconnected"));
}
} else if (native.closedIncoming && native.closedOutgoing) {
if (native.state.is<Accepted>()) {
if (awaitingHibernatableRelease()) {
// Because Hibernatable WebSocket events are dispatched at different times,
// we may have a situation where one event's pump() promise finishes (setting
// isPumping to false), but then this continuation doesn't get scheduled until after we
// start handling the next websocket event, which may call send()/close(), and result in
// another pump() promise being registered.
//
// When the first event continues execution, if we release the WebSocket without
// confirming it is not currently pumping, we can transition the WebSockets state from
// Accepted to Released. This will cancel the second events pump() promise.
tryReleaseNative(js);
} else if (native.state.is<Accepted>()) {
// Native WebSocket no longer needed; release.
native.state.init<Released>();
} else if (native.state.is<Released>()) {
Expand Down

0 comments on commit a0a979b

Please sign in to comment.