[WIP] Attempt to reproduce Windows compiler crashes

.bazelrc

@@ -357,7 +357,12 @@ build:windows --extra_execution_platforms=//:x64_windows-clang-cl
 # workaround.
 
 build:windows_no_dbg -c opt
-build:windows_no_dbg --copt='/Od'
+# build:windows_no_dbg --copt='/Od'
+# build:windows_no_dbg --per_file_copt=src/workerd/api@/Od
+build:windows_no_dbg --per_file_copt=src/workerd/io/worker-[e-i]@/Od
+# build:windows_no_dbg --per_file_copt=src/workerd/jsg@/Od
+# build:windows_no_dbg --per_file_copt=src/workerd/util@/Od
+# build:windows_no_dbg --per_file_copt=src/workerd/server@/Od
 build:windows_no_dbg --linkopt='/INCREMENTAL:NO'
 build:windows_no_dbg --features=-smaller_binary
 
@@ -431,7 +436,9 @@ build:release_windows --copt="/clang:-O3"
 build:release_windows --copt="-fstrict-aliasing"
 # This file breaks our CI windows release builds when compiled using O2/O3
 # Ref: https://github.com/llvm/llvm-project/issues/136481
-build:release_windows --per_file_copt=.*capnp/rpc\.c++@/clang:-O1
+build:windows --per_file_copt=.*capnp/rpc\.c++@"/O2,/Gy,/Gw,-fstrict-aliasing,/clang:-O3"
+build:windows --per_file_copt="src/workerd/server/server"@/O2,/Gy,/Gw,-fstrict-aliasing,/clang:-O3,/clang:-fno-inline
+build:windows --per_file_copt="src/workerd/server/container-client"@/O2,/Gy,/Gw,-fstrict-aliasing,/clang:-O3,/clang:-fno-inline
 
 build:windows --cxxopt='/std:c++23preview' --host_cxxopt='/std:c++23preview'
 build:windows --copt='/D_CRT_USE_BUILTIN_OFFSETOF' --host_copt='/D_CRT_USE_BUILTIN_OFFSETOF'

.github/workflows/_bazel.yml

@@ -126,7 +126,7 @@ jobs:
           bazel --nowindows_enable_symlinks build ${{ inputs.extra_bazel_args }} --config=ci --profile build-win-workaround.bazel-profile.gz --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev //src/wpt:wpt-all@tsproject //src/node:node@tsproject //src/pyodide:pyodide_static@tsproject
       - name: Bazel build
         run: |
-          bazel build --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev  --config=ci ${{ inputs.extra_bazel_args }} //...
+          bazel build --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev  --config=ci ${{ inputs.extra_bazel_args }} //src/workerd/api/tests/... //src/workerd/server:server-test@ //src/workerd/server:server-test@all-autogates
       - name: Configure Docker daemon for IPv6
         if: inputs.build_container_images
         run: |
@@ -145,7 +145,7 @@ jobs:
       - name: Bazel test
         if: inputs.run_tests
         run: |
-          bazel test --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=ci ${{ inputs.extra_bazel_args }} ${{ inputs.test_target }}
+          bazel test --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=ci ${{ inputs.extra_bazel_args }} //src/workerd/api/tests/... //src/workerd/server:server-test@ //src/workerd/server:server-test@all-autogates
       - name: Bazel coverage
         if: inputs.run_coverage
         run: |

src/workerd/api/actor.h

@@ -108,15 +108,14 @@ class DurableObject final: public Fetcher {
 
     JSG_TS_DEFINE(interface DurableObject {
       fetch(request: Request): Response | Promise<Response>;
-      connect?(socket: Socket): void | Promise<void>;
       alarm?(alarmInfo?: AlarmInvocationInfo): void | Promise<void>;
       webSocketMessage?(ws: WebSocket, message: string | ArrayBuffer): void | Promise<void>;
       webSocketClose?(ws: WebSocket, code: number, reason: string, wasClean: boolean): void | Promise<void>;
       webSocketError?(ws: WebSocket, error: unknown): void | Promise<void>;
     });
     JSG_TS_OVERRIDE(
       type DurableObjectStub<T extends Rpc.DurableObjectBranded | undefined = undefined> =
-        Fetcher<T, "alarm" | "connect" | "webSocketMessage" | "webSocketClose" | "webSocketError">
+        Fetcher<T, "alarm" | "webSocketMessage" | "webSocketClose" | "webSocketError">
         & {
           readonly id: DurableObjectId;
           readonly name?: string;

src/workerd/api/container.c++

@@ -8,32 +8,10 @@
 #include <workerd/io/features.h>
 #include <workerd/io/io-context.h>
 
-#include <kj/filesystem.h>
-
 #include <cmath>
 
 namespace workerd::api {
 
-namespace {
-
-kj::Maybe<kj::Path> parseRestorePath(kj::StringPtr path) {
-  JSG_REQUIRE(path.size() > 0 && path[0] == '/', TypeError,
-      "Directory snapshot restore path must be absolute. Got: ", path);
-
-  try {
-    auto parsed = kj::Path::parse(path.slice(1));
-    if (parsed.size() == 0) {
-      return kj::none;
-    }
-    return kj::mv(parsed);
-  } catch (kj::Exception&) {
-    JSG_FAIL_REQUIRE(
-        TypeError, "Directory snapshot restore path contains invalid components: ", path);
-  }
-}
-
-}  // namespace
-
 // =======================================================================================
 // Basic lifecycle methods
 
@@ -108,14 +86,6 @@ void Container::start(jsg::Lock& js, jsg::Optional<StartupOptions> maybeOptions)
         auto entry = list[i];
         auto& restore = snapshots[i];
         auto& snap = restore.snapshot;
-        auto effectiveRestoreDir = snap.dir.asPtr();
-        KJ_IF_SOME(mp, restore.mountPoint) {
-          effectiveRestoreDir = mp.asPtr();
-        }
-
-        JSG_REQUIRE_NONNULL(parseRestorePath(effectiveRestoreDir), Error,
-            "Directory snapshot cannot be restored to root directory.");
-
         double size = snap.size;
         JSG_REQUIRE(std::isfinite(size) && size >= 0 &&
                 size <= static_cast<double>(jsg::MAX_SAFE_INTEGER) && std::floor(size) == size,

src/workerd/api/global-scope.c++

@@ -15,7 +15,6 @@
 #endif
 #include <workerd/api/hibernatable-web-socket.h>
 #include <workerd/api/scheduled.h>
-#include <workerd/api/sockets.h>
 #include <workerd/api/system-streams.h>
 #include <workerd/api/trace.h>
 #include <workerd/api/util.h>
@@ -87,7 +86,6 @@ jsg::LenientOptional<T> mapAddRef(jsg::Lock& js, jsg::LenientOptional<T>& functi
 ExportedHandler ExportedHandler::clone(jsg::Lock& js) {
   return ExportedHandler{
     .fetch{mapAddRef(js, fetch)},
-    .connect{mapAddRef(js, connect)},
     .tail{mapAddRef(js, tail)},
     .trace{mapAddRef(js, trace)},
     .tailStream{mapAddRef(js, tailStream)},
@@ -120,49 +118,6 @@ void ServiceWorkerGlobalScope::clear() {
   unhandledRejections.clear();
 }
 
-kj::Promise<void> ServiceWorkerGlobalScope::connect(kj::String host,
-    const kj::HttpHeaders& headers,
-    kj::AsyncIoStream& connection,
-    kj::HttpService::ConnectResponse& response,
-    Worker::Lock& lock,
-    kj::Maybe<ExportedHandler&> exportedHandler) {
-  ExportedHandler& eh = JSG_REQUIRE_NONNULL(exportedHandler, Error,
-      "Connect ingress is not currently supported with Service Workers syntax.");
-  KJ_REQUIRE(FeatureFlags::get(lock).getWorkerdExperimental(),
-      "connect handling requires the experimental flag.");
-
-  KJ_IF_SOME(handler, eh.connect) {
-    // Has a connect handler!
-    response.accept(200, "OK", headers);
-
-    // Using neuterable stream to manage lifetime of stream promises
-    auto ownConnection = newNeuterableIoStream(connection);
-
-    auto& ioContext = IoContext::current();
-    jsg::Lock& js = lock;
-
-    // TLS support is not implemented so far. Note that setupSocket() expects the domain parameter
-    // to be set to the expected host name using startTLS, so that it can be provided to the TLS
-    // callback, so we'd need to change that or figure out a way to get the host domain.
-    auto nullTlsStarter = kj::heap<kj::TlsStarterCallback>();
-    // We set isDefaultFetchPort to false here – sockets.c++ sets it for ports 443 and 8080 to
-    // provide a more descriptive error message for HTTP, but this is not relevant on the TCP server
-    // side.
-    jsg::Ref<Socket> jsSocket = setupSocket(js, kj::mv(ownConnection), kj::none, kj::none,
-        kj::mv(nullTlsStarter), SecureTransportKind::OFF, kj::none, false, kj::none);
-    // handleProxyStatus() is required to indicate that the socket was opened properly. Since the
-    // connection is already open at this point, exception handling is not required.
-    jsSocket->handleProxyStatus(js, kj::Promise<kj::Maybe<kj::Exception>>(kj::none));
-
-    kj::Maybe<SpanBuilder> span = ioContext.makeTraceSpan("connect_handler"_kjc);
-    auto promise = handler(js, kj::mv(jsSocket), eh.env.addRef(js), eh.getCtx());
-    return ioContext.awaitJs(js, kj::mv(promise)).attach(kj::mv(span));
-  }
-  lock.logWarningOnce("Received a connect event but we lack a handler. "
-                      "Did you remember to export a connect() function?");
-  JSG_FAIL_REQUIRE(Error, "Handler does not export a connect() function.");
-}
-
 kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMethod method,
     kj::StringPtr url,
     const kj::HttpHeaders& headers,

src/workerd/api/global-scope.h

@@ -357,10 +357,6 @@ struct ExportedHandler {
       jsg::Optional<jsg::Ref<ExecutionContext>> ctx);
   jsg::LenientOptional<jsg::Function<FetchHandler>> fetch;
 
-  using ConnectHandler = jsg::Promise<void>(
-      jsg::Ref<Socket> socket, jsg::Value env, jsg::Optional<jsg::Ref<ExecutionContext>> ctx);
-  jsg::LenientOptional<jsg::Function<ConnectHandler>> connect;
-
   using TailHandler = kj::Promise<void>(kj::Array<jsg::Ref<TraceItem>> events,
       jsg::Value env,
       jsg::Optional<jsg::Ref<ExecutionContext>> ctx);
@@ -400,7 +396,6 @@ struct ExportedHandler {
   jsg::SelfRef self;
 
   JSG_STRUCT(fetch,
-      connect,
       tail,
       trace,
       tailStream,
@@ -418,7 +413,6 @@ struct ExportedHandler {
 
   JSG_STRUCT_TS_DEFINE(
     type ExportedHandlerFetchHandler<Env = unknown, CfHostMetadata = unknown, Props = unknown> = (request: Request<CfHostMetadata, IncomingRequestCfProperties<CfHostMetadata>>, env: Env, ctx: ExecutionContext<Props>) => Response | Promise<Response>;
-    type ExportedHandlerConnectHandler<Env = unknown, Props = unknown> = (socket: Socket, env: Env, ctx: ExecutionContext<Props>) => void | Promise<void>;
     type ExportedHandlerTailHandler<Env = unknown, Props = unknown> = (events: TraceItem[], env: Env, ctx: ExecutionContext<Props>) => void | Promise<void>;
     type ExportedHandlerTraceHandler<Env = unknown, Props = unknown> = (traces: TraceItem[], env: Env, ctx: ExecutionContext<Props>) => void | Promise<void>;
     type ExportedHandlerTailStreamHandler<Env = unknown, Props = unknown> = (event : TailStream.TailEvent<TailStream.Onset>, env: Env, ctx: ExecutionContext<Props>) => TailStream.TailEventHandlerType | Promise<TailStream.TailEventHandlerType>;
@@ -429,7 +423,6 @@ struct ExportedHandler {
   JSG_STRUCT_TS_OVERRIDE(<Env = unknown, QueueHandlerMessage = unknown, CfHostMetadata = unknown, Props = unknown> {
     email?: EmailExportedHandler<Env, Props>;
     fetch?: ExportedHandlerFetchHandler<Env, CfHostMetadata, Props>;
-    connect?: ExportedHandlerConnectHandler<Env, Props>;
     tail?: ExportedHandlerTailHandler<Env, Props>;
     trace?: ExportedHandlerTraceHandler<Env, Props>;
     tailStream?: ExportedHandlerTailStreamHandler<Env, Props>;
@@ -529,14 +522,6 @@ class ServiceWorkerGlobalScope: public WorkerGlobalScope {
   // TODO(cleanup): Factor out the shared code used between old-style event listeners vs. module
   //   exports and move that code somewhere more appropriate.
 
-  // Received TCP/socket ingress (called from C++, not JS).
-  kj::Promise<void> connect(kj::String host,
-      const kj::HttpHeaders& headers,
-      kj::AsyncIoStream& connection,
-      kj::HttpService::ConnectResponse& response,
-      Worker::Lock& lock,
-      kj::Maybe<ExportedHandler&> exportedHandler);
-
   // Received sendTraces (called from C++, not JS).
   void sendTraces(kj::ArrayPtr<kj::Own<Trace>> traces,
       Worker::Lock& lock,

src/workerd/api/sockets.c++

@@ -84,11 +84,11 @@ class StreamWorkerInterface;
 
 jsg::Ref<Socket> setupSocket(jsg::Lock& js,
     kj::Own<kj::AsyncIoStream> connection,
-    kj::Maybe<kj::String> remoteAddress,
+    kj::String remoteAddress,
     jsg::Optional<SocketOptions> options,
     kj::Own<kj::TlsStarterCallback> tlsStarter,
     SecureTransportKind secureTransport,
-    kj::Maybe<kj::String> domain,
+    kj::String domain,
     bool isDefaultFetchPort,
     kj::Maybe<jsg::PromiseResolverPair<SocketInfo>> maybeOpenedPrPair) {
   auto& ioContext = IoContext::current();
@@ -323,10 +323,10 @@ jsg::Ref<Socket> Socket::startTls(jsg::Lock& js, jsg::Optional<TlsOptions> tlsOp
       secureTransport != SecureTransportKind::ON, TypeError, "Cannot startTls on a TLS socket.");
   JSG_REQUIRE(connectionData != kj::none, TypeError,
       "The connection was closed before startTls could be started.");
+  JSG_REQUIRE(domain != nullptr, TypeError, "startTls can only be called once.");
   auto invalidOptKindMsg =
       "The `secureTransport` socket option must be set to 'starttls' for startTls to be used.";
   JSG_REQUIRE(secureTransport == SecureTransportKind::STARTTLS, TypeError, invalidOptKindMsg);
-  JSG_REQUIRE(domain != kj::none, TypeError, "startTls can only be called once.");
 
   // The current socket's writable buffers need to be flushed. The socket's WritableStream is backed
   // by an AsyncIoStream which doesn't implement any buffering, so we don't need to worry about
@@ -346,10 +346,10 @@ jsg::Ref<Socket> Socket::startTls(jsg::Lock& js, jsg::Optional<TlsOptions> tlsOp
           // flush to complete. While it is unlikely to be GC'd while we are waiting because
           // the user code *likely* is holding a active reference to it at this point, we
           // don't want to take any chances. This prevents a possible UAF.
-          JSG_VISITABLE_LAMBDA((self = JSG_THIS, domain = kj::heapString(KJ_ASSERT_NONNULL(domain)),
-                                   tlsOptions = kj::mv(tlsOptions),
-                                   openedResolver = openedPrPair.resolver.addRef(js),
-                                   remoteAddress = mapCopyString(remoteAddress)),
+          JSG_VISITABLE_LAMBDA(
+              (self = JSG_THIS, domain = kj::heapString(domain), tlsOptions = kj::mv(tlsOptions),
+                  openedResolver = openedPrPair.resolver.addRef(js),
+                  remoteAddress = kj::str(remoteAddress)),
               (self, openedResolver), (jsg::Lock & js) mutable {
                 auto& context = IoContext::current();
 
@@ -381,7 +381,7 @@ jsg::Ref<Socket> Socket::startTls(jsg::Lock& js, jsg::Optional<TlsOptions> tlsOp
 
                 // Fork the starter promise because we need to create two separate things waiting
                 // on it below. The first is resolving the openedResolver with a JS promise that
-                // wraps one branch, the second is the kj::Promise that we use to resolve the
+                // wraps one branch, the secnod is the kj::Promise that we use to resolve the
                 // secureStream for the promised stream. This keeps us from having to bounce in and
                 // out of the JS isolate lock.
                 auto forkedPromise = KJ_ASSERT_NONNULL(*tlsStarter)(acceptedHostname).fork();
@@ -410,9 +410,9 @@ jsg::Ref<Socket> Socket::startTls(jsg::Lock& js, jsg::Optional<TlsOptions> tlsOp
   // The existing tlsStarter gets consumed and we won't need it again. Pass in an empty tlsStarter
   // to `setupSocket`.
   auto newTlsStarter = kj::heap<kj::TlsStarterCallback>();
-  return setupSocket(js, kj::newPromisedStream(kj::mv(secureStreamPromise)),
-      mapCopyString(remoteAddress), kj::mv(options), kj::mv(newTlsStarter), SecureTransportKind::ON,
-      kj::mv(domain), isDefaultFetchPort, kj::mv(openedPrPair));
+  return setupSocket(js, kj::newPromisedStream(kj::mv(secureStreamPromise)), kj::str(remoteAddress),
+      kj::mv(options), kj::mv(newTlsStarter), SecureTransportKind::ON, kj::mv(domain),
+      isDefaultFetchPort, kj::mv(openedPrPair));
 }
 
 void Socket::handleProxyStatus(
@@ -436,7 +436,7 @@ void Socket::handleProxyStatus(
       if (isDefaultFetchPort) {
         msg = kj::str(msg, ". It looks like you might be trying to connect to a HTTP-based service",
             " — consider using fetch instead");
-      } else if (remoteAddress.orDefault(kj::String()).contains(".hyperdrive.local"_kj)) {
+      } else if (remoteAddress.contains(".hyperdrive.local"_kj)) {
         // No attempts to connect to Hyperdrive should end up here, since they go through the other
         // version of handleProxyStatus. If they end up here somehow, log about it to get some
         // context that can aid in debugging.
@@ -450,7 +450,7 @@ void Socket::handleProxyStatus(
       // because there's no useful value we can provide.
       openedResolver.resolve(js,
           SocketInfo{
-            .remoteAddress = mapCopyString(remoteAddress),
+            .remoteAddress = kj::str(remoteAddress),
             .localAddress = kj::none,
           });
     }
@@ -478,7 +478,7 @@ void Socket::handleProxyStatus(jsg::Lock& js, kj::Promise<kj::Maybe<kj::Exceptio
       // because there's no useful value we can provide.
       openedResolver.resolve(js,
           SocketInfo{
-            .remoteAddress = mapCopyString(remoteAddress),
+            .remoteAddress = kj::str(remoteAddress),
             .localAddress = kj::none,
           });
     }