Revert "Try to take lock when delivering websocket events"

cloudflare · Jan 30, 2024 · fe67bd8 · fe67bd8
1 parent fa6bac1
commit fe67bd8
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 16 deletions.
diff --git a/src/workerd/api/web-socket.c++ b/src/workerd/api/web-socket.c++
@@ -57,8 +57,7 @@ WebSocket::WebSocket(jsg::Lock& js,
               kj::mv(KJ_REQUIRE_NONNULL(package.maybeTags)),
               package.closedOutgoingConnection)),
       outgoingMessages(IoContext::current().addObject(kj::heap<OutgoingMessagesMap>())),
-      locality(LOCAL),
-      maybeCriticalSection(IoContext::current().getCriticalSection()) {}
+      locality(LOCAL) {}
   // This constructor is used when reinstantiating a websocket that had been hibernating, which is
   // why we can go straight to the Accepted state. However, note that we are actually in the
   // `Hibernatable` "sub-state"!
@@ -74,8 +73,7 @@ WebSocket::WebSocket(kj::Own<kj::WebSocket> native, Locality locality)
     : url(kj::none),
       farNative(nullptr),
       outgoingMessages(IoContext::current().addObject(kj::heap<OutgoingMessagesMap>())),
-      locality(locality),
-      maybeCriticalSection(IoContext::current().getCriticalSection()) {
+      locality(locality) {
   auto nativeObj = kj::heap<Native>();
   nativeObj->state.init<AwaitingAcceptanceOrCoupling>(kj::mv(native));
   farNative = IoContext::current().addObject(kj::mv(nativeObj));
@@ -85,8 +83,7 @@ WebSocket::WebSocket(kj::String url, Locality locality)
     : url(kj::mv(url)),
       farNative(nullptr),
       outgoingMessages(IoContext::current().addObject(kj::heap<OutgoingMessagesMap>())),
-      locality(locality),
-      maybeCriticalSection(IoContext::current().getCriticalSection()) {
+      locality(locality) {
   auto nativeObj = kj::heap<Native>();
   nativeObj->state.init<AwaitingConnection>();
   farNative = IoContext::current().addObject(kj::mv(nativeObj));
@@ -919,11 +916,6 @@ kj::Promise<kj::Maybe<kj::Exception>> WebSocket::readLoop() {
         a.getMetrics().receivedWebSocketMessage(size);
       }
 
-      // If we called from a blockConcurrencyWhile callback, we need to get the critical section
-      // lock so this event counts as part of the callback. If we run it as a separate event,
-      // it will need to wait for the blockConcurrencyWhile to finish, but that event will be
-      // waiting on this new event resulting in a deadlock.
-      //
       // Re-enter the context with context.run(). This is arguably a bit unusual compared to other
       // I/O which is delivered by return from context.awaitIo(), but the difference here is that we
       // have a long stream of events over time. It makes sense to use context.run() each time a new
@@ -955,7 +947,7 @@ kj::Promise<kj::Maybe<kj::Exception>> WebSocket::readLoop() {
         }
 
         return true;
-      }, mapAddRef(maybeCriticalSection));
+      });
 
       if (!result) co_return kj::none;
     }

diff --git a/src/workerd/api/web-socket.h b/src/workerd/api/web-socket.h
@@ -608,10 +608,6 @@ class WebSocket: public EventTarget {
     kj::Maybe<kj::String> extensions;
   };
 
-  // If we created this WebSocket inside a critical section (ex. a blockConcurrencyWhile callback)
-  // then we need to get the InputGate::Lock and pass it to context.run() when delivering events.
-  kj::Maybe<InputGate::CriticalSection&> maybeCriticalSection;
-
   // So that each end of a WebSocketPair can keep track of its pair.
   kj::Maybe<jsg::Ref<WebSocket>> maybePair;