Skip to content

Commit

Permalink
Merge pull request #1660 from cloudflare/dominik/remove-old-socket-close
Browse files Browse the repository at this point in the history
Removes old Socket::close implementation.
  • Loading branch information
dom96 committed Feb 13, 2024
2 parents 5392948 + f77eb6b commit b3ed665
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 39 deletions.
36 changes: 1 addition & 35 deletions src/workerd/api/sockets.c++
Original file line number Diff line number Diff line change
Expand Up @@ -263,33 +263,7 @@ jsg::Ref<Socket> connectImpl(
return connectImplNoOutputLock(js, kj::mv(fetcher), kj::mv(address), kj::mv(options));
}

// Closes the underlying socket connection. This is an old implementation and will be removed soon.
// See closeImplNew below for the new implementation.
//
// TODO(later): remove once safe
jsg::Promise<void> Socket::closeImplOld(jsg::Lock& js) {
// Forcibly close the readable/writable streams.
auto cancelPromise = readable->getController().cancel(js, kj::none);
auto abortPromise = writable->getController().abort(js, kj::none);
// The below is effectively `Promise.all(cancelPromise, abortPromise)`
return cancelPromise.then(js, [abortPromise = kj::mv(abortPromise), this](jsg::Lock& js) mutable {
return abortPromise.then(js, [this](jsg::Lock& js) {
resolveFulfiller(js, kj::none);
return js.resolvedPromise();
}, [this](jsg::Lock& js, jsg::Value err) {
errorHandler(js, kj::mv(err));
return js.resolvedPromise();
});
}, [this](jsg::Lock& js, jsg::Value err) {
errorHandler(js, kj::mv(err));
return js.resolvedPromise();
});
}

// Closes the underlying socket connection, but only after the socket connection is properly
// established through any configured proxy. This method also flushes the writable stream prior to
// closing.
jsg::Promise<void> Socket::closeImplNew(jsg::Lock& js) {
jsg::Promise<void> Socket::close(jsg::Lock& js) {
if (isClosing) {
return closedPromiseCopy.whenResolved(js);
}
Expand Down Expand Up @@ -322,14 +296,6 @@ jsg::Promise<void> Socket::closeImplNew(jsg::Lock& js) {
});
}

jsg::Promise<void> Socket::close(jsg::Lock& js) {
if (util::Autogate::isEnabled(util::AutogateKey::SOCKETS_AWAIT_PROXY_BEFORE_CLOSE)) {
return closeImplNew(js);
} else {
return closeImplOld(js);
}
}

jsg::Ref<Socket> Socket::startTls(jsg::Lock& js, jsg::Optional<TlsOptions> tlsOptions) {
JSG_REQUIRE(!isSecureSocket, TypeError, "Cannot startTls on a TLS socket.");
// TODO: Track closed state of socket properly and assert that it hasn't been closed here.
Expand Down
4 changes: 4 additions & 0 deletions src/workerd/api/sockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ class Socket: public jsg::Object {
}

// Closes the socket connection.
//
// The closure is only performed after the socket connection is properly
// established through any configured proxy. This method also flushes the writable stream prior to
// closing.
jsg::Promise<void> close(jsg::Lock& js);

// Flushes write buffers then performs a TLS handshake on the current Socket connection.
Expand Down
2 changes: 0 additions & 2 deletions src/workerd/util/autogate.c++
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ kj::StringPtr KJ_STRINGIFY(AutogateKey key) {
switch (key) {
case AutogateKey::TEST_WORKERD:
return "test-workerd"_kj;
case AutogateKey::SOCKETS_AWAIT_PROXY_BEFORE_CLOSE:
return "sockets-await-proxy-before-close"_kj;
case AutogateKey::NumOfKeys:
KJ_FAIL_ASSERT("NumOfKeys should not be used in getName");
}
Expand Down
2 changes: 0 additions & 2 deletions src/workerd/util/autogate.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ namespace workerd::util {
// Workerd-specific list of autogate keys (can also be used in internal repo).
enum class AutogateKey {
TEST_WORKERD,
// Enable new behaviour of Socket::close (specifically waiting for proxy result before closing).
SOCKETS_AWAIT_PROXY_BEFORE_CLOSE,
NumOfKeys // Reserved for iteration.
};

Expand Down

0 comments on commit b3ed665

Please sign in to comment.