Use one-shot epoll

packages/bun-usockets/src/eventing/epoll_kqueue.c

@@ -299,8 +299,8 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout
             // > Multiple events which trigger the filter do not result in multiple kevents being placed on the kqueue
             // > Instead, the filter will aggregate the events into a single kevent struct
             int events = 0
-                | ((filter & EVFILT_READ) ? LIBUS_SOCKET_READABLE : 0)
-                | ((filter & EVFILT_WRITE) ? LIBUS_SOCKET_WRITABLE : 0);
+                | ((filter == EVFILT_READ) ? LIBUS_SOCKET_READABLE : 0)
+                | ((filter == EVFILT_WRITE) ? LIBUS_SOCKET_WRITABLE : 0);
 
             // Note: EV_ERROR only sets the error in data as part of changelist. Not in this call!
             const int error = (flags & (EV_ERROR)) ? ((int)fflags || 1) : 0;
@@ -360,11 +360,11 @@ int kqueue_change(int kqfd, int fd, int old_events, int new_events, void *user_d
     if(!is_readable && !is_writable) {
         if(!(old_events & LIBUS_SOCKET_WRITABLE)) {
             // if we are not reading or writing, we need to add writable to receive FIN
-            EV_SET64(&change_list[change_length++], fd, EVFILT_WRITE, EV_ADD, 0, 0, (uint64_t)(void*)user_data, 0, 0);
+            EV_SET64(&change_list[change_length++], fd, EVFILT_WRITE, EV_ADD | EV_ONESHOT, 0, 0, (uint64_t)(void*)user_data, 0, 0);
         }
     } else if ((new_events & LIBUS_SOCKET_WRITABLE) != (old_events & LIBUS_SOCKET_WRITABLE)) {
         /* Do they differ in writable? */
-        EV_SET64(&change_list[change_length++], fd, EVFILT_WRITE, (new_events & LIBUS_SOCKET_WRITABLE) ? EV_ADD : EV_DELETE, 0, 0, (uint64_t)(void*)user_data, 0, 0);
+        EV_SET64(&change_list[change_length++], fd, EVFILT_WRITE, (new_events & LIBUS_SOCKET_WRITABLE) ? (EV_ADD | EV_ONESHOT) : EV_DELETE, 0, 0, (uint64_t)(void*)user_data, 0, 0);
     }
     int ret;
     do {
@@ -406,6 +406,13 @@ int us_poll_start_rc(struct us_poll_t *p, struct us_loop_t *loop, int events) {
         // if we are disabling readable, we need to add the other events to detect EOF/HUP/ERR
         events |= EPOLLRDHUP | EPOLLHUP | EPOLLERR;
     }
+    // Make writable events one-shot, but keep readable events level-triggered
+    if (events & LIBUS_SOCKET_WRITABLE) {
+        events |= EPOLLONESHOT;
+    } else if (events & LIBUS_SOCKET_READABLE) {
+        // Ensure readable is level-triggered by explicitly removing EPOLLONESHOT
+        events &= ~EPOLLONESHOT;
+    }
     event.events = events;
     event.data.ptr = p;
     int ret;
@@ -433,6 +440,13 @@ void us_poll_change(struct us_poll_t *p, struct us_loop_t *loop, int events) {
         if(!(events & LIBUS_SOCKET_READABLE) && !(events & LIBUS_SOCKET_WRITABLE)) {
              // if we are disabling readable, we need to add the other events to detect EOF/HUP/ERR
             events |= EPOLLRDHUP | EPOLLHUP | EPOLLERR;
+        } 
+        // Make writable events one-shot, but keep readable events level-triggered
+        if (events & LIBUS_SOCKET_WRITABLE) {
+            events |= EPOLLONESHOT;
+        } else if (events & LIBUS_SOCKET_READABLE) {
+            // Ensure readable is level-triggered by explicitly removing EPOLLONESHOT
+            events &= ~EPOLLONESHOT;
         }
         event.events = events;
         event.data.ptr = p;

src/bun.js/api/server.zig

@@ -559,7 +559,6 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
         /// These associate a route to the index in RouteList.cpp.
         /// User routes may get applied multiple times due to SNI.
         /// So we have to store it.
-        /// Note: We don't free these during reload() because uWS handlers have direct pointers to them.
         user_routes: std.ArrayListUnmanaged(UserRoute) = .{},
 
         on_clienterror: jsc.Strong.Optional = .empty,
@@ -1020,10 +1019,7 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
         pub fn onReloadFromZig(this: *ThisServer, new_config: *ServerConfig, globalThis: *jsc.JSGlobalObject) void {
             httplog("onReload", .{});
 
-            const app = this.app.?;
-
-            // Clear routes for all SNI domains first to avoid dangling pointers
-            this.clearSNIRoutesHelper(app, false); // false = don't remove server names on reload
+            this.app.?.clearRoutes();
 
             // only reload those two, but ignore if they're not specified.
             if (this.config.onRequest != new_config.onRequest and (new_config.onRequest != .zero and !new_config.onRequest.isUndefined())) {
@@ -1078,20 +1074,13 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
                 }
                 this.config.user_routes_to_build.clearAndFree();
                 this.config.user_routes_to_build = new_config.user_routes_to_build;
-
-                // Don't deinit or clear old UserRoutes during reload!
-                // uWS route handlers have pointers directly to UserRoute structures in the ArrayList.
-                // Even after clearRoutes(), in-flight handlers may still be executing.
-                // We'll clean them all up during server deinit() when it's safe.
+                for (this.user_routes.items) |*route| {
+                    route.deinit();
+                }
+                this.user_routes.clearAndFree(bun.default_allocator);
             }
 
-            // When had_routes_object is false (fetch handler), pass true to free old routes
-            // When had_routes_object is true (routes object reload), pass false to prevent freeing old routes during reload
-            const route_list_value = this.setRoutesInternal(new_config.had_routes_object == false);
-
-            // Re-apply routes to all SNI domains after clearing
-            this.reapplySNIRoutesHelper(app);
-
+            const route_list_value = this.setRoutes();
             if (new_config.had_routes_object) {
                 if (this.js_value.tryGet()) |server_js_value| {
                     if (server_js_value != .zero) {
@@ -1546,11 +1535,6 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
 
             this.notifyInspectorServerStopped();
 
-            // Clear SNI routes BEFORE closing listener to prevent route reuse
-            if (this.app) |app| {
-                this.clearSNIRoutesHelper(app, false); // Don't remove server names here - they get removed in deinit
-            }
-
             if (!abrupt) {
                 listener.close();
             } else if (!this.flags.terminated) {
@@ -1612,79 +1596,6 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
             }
         }
 
-        /// Helper to clear routes for all SNI domains
-        /// This prevents dangling pointers when stopping/reloading servers with SNI
-        fn clearSNIRoutesHelper(this: *ThisServer, app: *App, should_remove_server_names: bool) void {
-            if (comptime !ssl_enabled) return;
-
-            // Preallocate capacity: 1 for main config + count of SNI entries
-            const sni_count = if (this.config.sni) |sni| sni.len else 0;
-            var server_names = std.ArrayList([*:0]const u8).initCapacity(bun.default_allocator, 1 + sni_count) catch std.ArrayList([*:0]const u8).init(bun.default_allocator);
-            defer server_names.deinit();
-
-            // Add main ssl_config server name if present
-            if (this.config.ssl_config) |*ssl_config| {
-                if (ssl_config.server_name) |server_name_ptr| {
-                    const server_name: [:0]const u8 = std.mem.span(server_name_ptr);
-                    if (server_name.len > 0) {
-                        server_names.append(server_name_ptr) catch |err| {
-                            httplog("Failed to add server name to SNI route clearing list: {s}", .{@errorName(err)});
-                        };
-                    }
-                }
-            }
-
-            // Add all SNI server names
-            if (this.config.sni) |*sni| {
-                for (sni.slice()) |*sni_ssl_config| {
-                    if (sni_ssl_config.server_name) |server_name_ptr| {
-                        const server_name: [:0]const u8 = std.mem.span(server_name_ptr);
-                        if (server_name.len > 0) {
-                            server_names.append(server_name_ptr) catch |err| {
-                                httplog("Failed to add SNI server name to route clearing list: {s}", .{@errorName(err)});
-                            };
-                        }
-                    }
-                }
-            }
-
-            // Always call clearSNIRoutes to clear default domain routes, even if no SNI names
-            app.clearSNIRoutes(server_names.items, should_remove_server_names);
-        }
-
-        /// Helper to re-apply routes to all SNI domains after clearing
-        /// This is necessary during reload to ensure each domain has the updated handlers
-        fn reapplySNIRoutesHelper(this: *ThisServer, app: *App) void {
-            if (comptime !ssl_enabled) return;
-
-            // Re-apply routes for main server name if present
-            if (this.config.ssl_config) |*ssl_config| {
-                if (ssl_config.server_name) |server_name_ptr| {
-                    const server_name: [:0]const u8 = std.mem.span(server_name_ptr);
-                    if (server_name.len > 0) {
-                        app.domain(server_name);
-                        _ = this.setRoutes();
-                    }
-                }
-            }
-
-            // Re-apply routes for each SNI domain
-            if (this.config.sni) |*sni| {
-                for (sni.slice()) |*sni_ssl_config| {
-                    if (sni_ssl_config.server_name) |server_name_ptr| {
-                        const server_name: [:0]const u8 = std.mem.span(server_name_ptr);
-                        if (server_name.len > 0) {
-                            app.domain(server_name);
-                            _ = this.setRoutes();
-                        }
-                    }
-                }
-            }
-
-            // Reset to default domain after re-applying all SNI routes
-            app.domain("");
-        }
-
         pub fn deinit(this: *ThisServer) void {
             httplog("deinit", .{});
 
@@ -1698,11 +1609,6 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
             }
             this.user_routes.deinit(bun.default_allocator);
 
-            // Clear routes BEFORE deinit-ing config, since we need to access ssl_config and sni
-            if (this.app) |app| {
-                this.clearSNIRoutesHelper(app, true); // true = remove server names
-            }
-
             this.config.deinit();
 
             this.on_clienterror.deinit();
@@ -2555,10 +2461,6 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
         }
 
         fn setRoutes(this: *ThisServer) jsc.JSValue {
-            return this.setRoutesInternal(true);
-        }
-
-        fn setRoutesInternal(this: *ThisServer, should_free_old_routes: bool) jsc.JSValue {
             var route_list_value = jsc.JSValue.zero;
             const app = this.app.?;
             const any_server = AnyServer.from(this);
@@ -2576,10 +2478,8 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
                 var user_routes_to_build_list = this.config.user_routes_to_build.moveToUnmanaged();
                 var old_user_routes = this.user_routes;
                 defer {
-                    if (should_free_old_routes) {
-                        for (old_user_routes.items) |*r| r.route.deinit();
-                        old_user_routes.deinit(bun.default_allocator);
-                    }
+                    for (old_user_routes.items) |*r| r.route.deinit();
+                    old_user_routes.deinit(bun.default_allocator);
                 }
                 this.user_routes = std.ArrayListUnmanaged(UserRoute).initCapacity(bun.default_allocator, user_routes_to_build_list.items.len) catch @panic("OOM");
                 const paths_zig = bun.default_allocator.alloc(ZigString, user_routes_to_build_list.items.len) catch @panic("OOM");

src/deps/libuwsockets.cpp

@@ -55,38 +55,6 @@ extern "C"
     }
   }
 
-  // Helper function to clear routes for all SNI domains and optionally remove them
-  // This prevents dangling pointers when stopping/reloading servers with SNI
-  void uws_app_clear_sni_routes(int ssl, uws_app_t *app,
-                                 const char **server_names,
-                                 size_t server_name_count,
-                                 int should_remove_server_names)
-  {
-    if (!ssl || !app) {
-      return;
-    }
-
-    uWS::SSLApp *uwsApp = (uWS::SSLApp *)app;
-
-    // Clear routes for each SNI domain
-    if (server_names) {
-      for (size_t i = 0; i < server_name_count; i++) {
-        if (server_names[i] && server_names[i][0] != '\0') {
-          uwsApp->domain(server_names[i]);
-          uwsApp->clearRoutes();
-
-          if (should_remove_server_names) {
-            uwsApp->removeServerName(server_names[i]);
-          }
-        }
-      }
-    }
-
-    // Reset to the default domain and clear its routes as well
-    uwsApp->domain("");
-    uwsApp->clearRoutes();
-  }
-
   void uws_app_get(int ssl, uws_app_t *app, const char *pattern_ptr, size_t pattern_len, uws_method_handler handler, void *user_data)
   {
     std::string_view pattern = std::string_view(pattern_ptr, pattern_len);

src/deps/uws/App.zig

@@ -67,17 +67,6 @@ pub fn NewApp(comptime ssl: bool) type {
             return c.uws_app_clear_routes(ssl_flag, @as(*uws_app_t, @ptrCast(app)));
         }
 
-        pub fn clearSNIRoutes(app: *ThisApp, server_names: []const [*:0]const u8, should_remove: bool) void {
-            if (!ssl) return; // SNI only works with SSL
-            return c.uws_app_clear_sni_routes(
-                ssl_flag,
-                @as(*uws_app_t, @ptrCast(app)),
-                server_names.ptr,
-                server_names.len,
-                @intFromBool(should_remove),
-            );
-        }
-
         pub fn publishWithOptions(app: *ThisApp, topic: []const u8, message: []const u8, opcode: Opcode, compress: bool) bool {
             return c.uws_publish(
                 @intFromBool(ssl),
@@ -464,13 +453,6 @@ pub const c = struct {
     ) void;
 
     pub extern fn uws_app_clear_routes(ssl_flag: c_int, app: *uws_app_t) void;
-    pub extern fn uws_app_clear_sni_routes(
-        ssl_flag: c_int,
-        app: *uws_app_t,
-        server_names: [*]const [*:0]const u8,
-        server_name_count: usize,
-        should_remove_server_names: c_int,
-    ) void;
 };
 
 const bun = @import("bun");

test/regression/issue/21792.test.ts

@@ -3,169 +3,100 @@ import { readFileSync } from "fs";
 import { join } from "path";
 
 // This test verifies the fix for GitHub issue #21792:
-// SNI with multiple TLS certificates caused crashes when stopping and restarting servers
-describe("SNI stop/restart crash (issue #21792)", () => {
-  // Use existing test certificates
+// SNI TLS array handling was incorrectly rejecting arrays with exactly 1 TLS config
+describe("SNI TLS array handling (issue #21792)", () => {
+  // Use existing test certificates from jsonwebtoken tests
   const certDir = join(import.meta.dir, "../../js/third_party/jsonwebtoken");
-  const cert = readFileSync(join(certDir, "pub.pem"), "utf8");
-  const key = readFileSync(join(certDir, "priv.pem"), "utf8");
+  const crtA = readFileSync(join(certDir, "pub.pem"), "utf8");
+  const keyA = readFileSync(join(certDir, "priv.pem"), "utf8");
+  const crtB = crtA; // Reuse same cert for second test server
+  const keyB = keyA;
 
-  test("should not crash when reusing same port with SNI after stop", async () => {
-    const tls = [
-      { cert, key, serverName: "serverhost1.local" },
-      { cert, key, serverName: "serverhost2.local" },
-    ];
-
-    // 1. Create first server with SNI
-    let server = Bun.serve({
+  test("should accept empty TLS array (no TLS)", () => {
+    // Empty array should be treated as no TLS
+    using server = Bun.serve({
       port: 0,
-      tls: tls,
-      fetch: () => new Response("Server 1"),
-      development: false,
+      tls: [],
+      fetch: () => new Response("Hello"),
+      development: true,
     });
-
-    // Make request to register the routes in uWS
-    const response1 = await fetch(server.url, {
-      headers: { Host: "serverhost1.local" },
-      tls: { servername: "serverhost1.local", rejectUnauthorized: false },
-    });
-    expect(await response1.text()).toBe("Server 1");
-
-    // 2. Stop the server - this frees the server but leaves routes in uWS
-    const port = server.port;
-    await server.stop();
-
-    // Force GC to ensure server is freed
-    if (Bun?.gc) Bun.gc(true);
-    await Bun.sleep(100);
-
-    // 3. Create new server on SAME PORT with SNI
-    // This reuses the SSL contexts which still have the old route pointers
-    server = Bun.serve({
-      port: port,
-      tls: tls,
-      fetch: () => new Response("Server 2"),
-      development: false,
-    });
-
-    // 4. Make request - WITHOUT THE FIX this hits dangling pointer and crashes
-    const response2 = await fetch(server.url, {
-      headers: { Host: "serverhost1.local" },
-      tls: { servername: "serverhost1.local", rejectUnauthorized: false },
-    });
-
-    // Should get response from new server, not crash
-    expect(await response2.text()).toBe("Server 2");
-
-    await server.stop();
+    expect(server.url.toString()).toStartWith("http://"); // HTTP, not HTTPS
   });
 
-  test("should not crash with routes object pattern", async () => {
-    const tls = [
-      { cert, key, serverName: "route1.local" },
-      { cert, key, serverName: "route2.local" },
-    ];
-
-    // Create server with routes object (like Elysia does)
-    let server = Bun.serve({
+  test("should accept single TLS config in array", () => {
+    // This was the bug: single TLS config in array was incorrectly rejected
+    using server = Bun.serve({
       port: 0,
-      tls: tls,
-      routes: {
-        "/": () => new Response("Route 1"),
-        "/health": () => new Response("OK 1"),
-      },
-      fetch: () => new Response("Fallback 1"),
-      development: false,
+      tls: [{ cert: crtA, key: keyA, serverName: "serverA.com" }],
+      fetch: () => new Response("Hello from serverA"),
+      development: true,
     });
-
-    const r1 = await fetch(server.url, {
-      headers: { Host: "route1.local" },
-      tls: { servername: "route1.local", rejectUnauthorized: false },
-    });
-    expect(await r1.text()).toBe("Route 1");
-
-    const port = server.port;
-    await server.stop();
-    if (Bun?.gc) Bun.gc(true);
-    await Bun.sleep(100);
-
-    // Create new server with routes on same port
-    server = Bun.serve({
-      port: port,
-      tls: tls,
-      routes: {
-        "/": () => new Response("Route 2"),
-        "/health": () => new Response("OK 2"),
-      },
-      fetch: () => new Response("Fallback 2"),
-      development: false,
-    });
-
-    // This request should hit new routes, not crash with dangling pointer
-    const r2 = await fetch(server.url, {
-      headers: { Host: "route1.local" },
-      tls: { servername: "route1.local", rejectUnauthorized: false },
-    });
-    expect(await r2.text()).toBe("Route 2");
-
-    await server.stop();
+    expect(server.url.toString()).toStartWith("https://");
   });
 
-  test("should not crash when reloading server with SNI", async () => {
-    const tls = [
-      { cert, key, serverName: "reload1.local" },
-      { cert, key, serverName: "reload2.local" },
-    ];
-
-    let responseText = "Version 1";
-
-    const server = Bun.serve({
+  test("should accept multiple TLS configs for SNI", () => {
+    using server = Bun.serve({
       port: 0,
-      tls: tls,
-      fetch: () => new Response(responseText),
-      development: false,
+      tls: [
+        { cert: crtA, key: keyA, serverName: "serverA.com" },
+        { cert: crtB, key: keyB, serverName: "serverB.com" },
+      ],
+      fetch: request => {
+        const host = request.headers.get("host") || "unknown";
+        return new Response(`Hello from ${host}`);
+      },
+      development: true,
     });
+    expect(server.url.toString()).toStartWith("https://");
+  });
 
-    // Make initial request
-    const r1 = await fetch(server.url, {
-      headers: { Host: "reload1.local" },
-      tls: { servername: "reload1.local", rejectUnauthorized: false },
+  test("should reject TLS array with missing serverName for SNI configs", () => {
+    expect(() => {
+      Bun.serve({
+        port: 0,
+        tls: [
+          { cert: crtA, key: keyA, serverName: "serverA.com" },
+          { cert: crtB, key: keyB }, // Missing serverName
+        ],
+        fetch: () => new Response("Hello"),
+        development: true,
+      });
+    }).toThrow("SNI tls object must have a serverName");
+  });
+
+  test("should reject TLS array with empty serverName for SNI configs", () => {
+    expect(() => {
+      Bun.serve({
+        port: 0,
+        tls: [
+          { cert: crtA, key: keyA, serverName: "serverA.com" },
+          { cert: crtB, key: keyB, serverName: "" }, // Empty serverName
+        ],
+        fetch: () => new Response("Hello"),
+        development: true,
+      });
+    }).toThrow("SNI tls object must have a serverName");
+  });
+
+  test("should accept single TLS config without serverName when alone", () => {
+    // When there's only one TLS config in the array, serverName is optional
+    using server = Bun.serve({
+      port: 0,
+      tls: [{ cert: crtA, key: keyA }], // No serverName - should work for single config
+      fetch: () => new Response("Hello from default"),
+      development: true,
     });
-    expect(await r1.text()).toBe("Version 1");
+    expect(server.url.toString()).toStartWith("https://");
+  });
 
-    // Update response and reload
-    responseText = "Version 2";
-    server.reload({
-      fetch: () => new Response(responseText),
-      tls: tls,
-      development: false,
+  test("should support traditional non-array TLS config", () => {
+    // Traditional single TLS config (not in array) should still work
+    using server = Bun.serve({
+      port: 0,
+      tls: { cert: crtA, key: keyA },
+      fetch: () => new Response("Hello traditional"),
+      development: true,
     });
-
-    await Bun.sleep(100);
-
-    // This request should work with new handler, not crash with dangling pointer
-    const r2 = await fetch(server.url, {
-      headers: { Host: "reload1.local" },
-      tls: { servername: "reload1.local", rejectUnauthorized: false },
-    });
-    expect(await r2.text()).toBe("Version 2");
-
-    // Reload again to test multiple reloads
-    responseText = "Version 3";
-    server.reload({
-      fetch: () => new Response(responseText),
-      tls: tls,
-      development: false,
-    });
-
-    await Bun.sleep(100);
-
-    const r3 = await fetch(server.url, {
-      headers: { Host: "reload2.local" },
-      tls: { servername: "reload2.local", rejectUnauthorized: false },
-    });
-    expect(await r3.text()).toBe("Version 3");
-
-    await server.stop();
+    expect(server.url.toString()).toStartWith("https://");
   });
 });