Skip to content

Commit

Permalink
Additional websocket cleanup
Browse files Browse the repository at this point in the history
Related to the hibernation work; we noticed a few other things that
could potentially lead to problems.
  • Loading branch information
MellowYarker committed May 18, 2023
1 parent b81c9ca commit 6a32783
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/workerd/api/web-socket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ void WebSocket::ensurePumping(jsg::Lock& js) {
KJ_FAIL_ASSERT("Unexpected native web socket state", native.state);
}
}
}, [this](jsg::Lock& js, jsg::Value&& exception) mutable {
}, [this, thisHandle = JSG_THIS](jsg::Lock& js, jsg::Value&& exception) mutable {
if (awaitingHibernatableRelease()) {
// We have a hibernatable websocket -- we don't want to dispatch a regular error event.
tryReleaseNative(js);
Expand Down
12 changes: 6 additions & 6 deletions src/workerd/api/web-socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -556,18 +556,18 @@ class WebSocket: public EventTarget {
return ws.getIfNotHibernatable() == nullptr;
}

kj::Promise<void> createAbortTask(Native& native, IoContext& context);
kj::Promise<void> whenAbortedTask = nullptr;
// Listens for ws->whenAborted() and possibly triggers a proactive shutdown.

kj::Maybe<kj::Own<ActorObserver>> actorMetrics;

kj::Canceler canceler;
// This canceler wraps the pump loop as a precaution to make sure we can't exit the Accepted
// state with a pump task still happening asychronously. In practice the canceler should usually
// be empty when destroyed because we do not leave the Accepted state if we're still pumping.
// Even in the case of IoContext premature cancellation, the pump task should be canceled
// by the IoContext before the Canceler is destroyed.

kj::Promise<void> createAbortTask(Native& native, IoContext& context);
kj::Promise<void> whenAbortedTask = nullptr;
// Listens for ws->whenAborted() and possibly triggers a proactive shutdown.

kj::Maybe<kj::Own<ActorObserver>> actorMetrics;
};

struct Released {};
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/hibernation-manager.c++
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace workerd {

HibernationManagerImpl::~HibernationManagerImpl() {
HibernationManagerImpl::~HibernationManagerImpl() noexcept(false) {
// Note that the HibernatableWebSocket destructor handles removing any references to itself in
// `tagToWs`, and even removes the hashmap entry if there are no more entries in the bucket.
allWs.clear();
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/hibernation-manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class HibernationManagerImpl final : public Worker::Actor::HibernationManager {
hibernationEventType(hibernationEventType),
onDisconnect(DisconnectHandler{}),
readLoopTasks(onDisconnect) {}
~HibernationManagerImpl();
~HibernationManagerImpl() noexcept(false);

void acceptWebSocket(jsg::Ref<api::WebSocket> ws, kj::ArrayPtr<kj::String> tags) override;
// Tells the HibernationManager to create a new HibernatableWebSocket with the associated tags
Expand Down Expand Up @@ -73,7 +73,7 @@ class HibernationManagerImpl final : public Worker::Actor::HibernationManager {
ws(activeOrPackage.get<jsg::Ref<api::WebSocket>>()->acceptAsHibernatable()),
manager(manager) {}

~HibernatableWebSocket() {
~HibernatableWebSocket() noexcept(false) {
// We expect this dtor to be called when we're removing a HibernatableWebSocket
// from our `allWs` collection in the HibernationManager.

Expand Down

0 comments on commit 6a32783

Please sign in to comment.