socket: hold Bun.listen/Bun.connect callbacks in a GC-visited internal-fields cell

src/boringssl/Cargo.toml

@@ -12,7 +12,6 @@ workspace = true
 [dependencies]
 strum.workspace = true
 bstr.workspace = true
-scopeguard.workspace = true
 const_format.workspace = true
 enum-map.workspace = true
 enumset.workspace = true

src/boringssl/lib.rs

@@ -353,46 +353,37 @@ pub fn check_x509_server_identity(x509: &mut boring::X509, hostname: &[u8]) -> b
                     None
                 };
 
-                let names_ = boring::X509V3_EXT_d2i(ext);
-                if !names_.is_null() {
-                    let names = names_.cast::<boring::struct_stack_st_GENERAL_NAME>();
-                    let _guard = scopeguard::guard(names, |n| {
-                        boring::sk_GENERAL_NAME_pop_free(n, boring::sk_GENERAL_NAME_free)
-                    });
-                    for i in 0..boring::sk_GENERAL_NAME_num(names) {
-                        let r#gen = boring::sk_GENERAL_NAME_value(names, i);
-                        if let Some(name) = r#gen.as_ref() {
-                            match name.name_type {
-                                boring::GEN_URI => {
-                                    has_identifier_san = true;
-                                }
-                                boring::GEN_DNS => {
-                                    has_identifier_san = true;
-                                    if !host_is_ip {
-                                        let dns_name = &*name.d.dNSName;
-                                        let dns_name_slice = core::slice::from_raw_parts(
-                                            dns_name.data,
-                                            usize::try_from(dns_name.length).expect("int cast"),
-                                        );
-                                        if match_dns_name(dns_name_slice, hostname) {
-                                            return true;
-                                        }
+                if let Some(names) = boring::GeneralNames::from_raw(boring::X509V3_EXT_d2i(ext)) {
+                    for name in names.iter() {
+                        match name.name_type {
+                            boring::GEN_URI => {
+                                has_identifier_san = true;
+                            }
+                            boring::GEN_DNS => {
+                                has_identifier_san = true;
+                                if !host_is_ip {
+                                    let dns_name = &*name.d.dNSName;
+                                    let dns_name_slice = core::slice::from_raw_parts(
+                                        dns_name.data,
+                                        usize::try_from(dns_name.length).expect("int cast"),
+                                    );
+                                    if match_dns_name(dns_name_slice, hostname) {
+                                        return true;
                                     }
                                 }
-                                boring::GEN_IPADD => {
-                                    has_identifier_san = true;
-                                    if let Some(hip) = host_ip {
-                                        if let Some(cert_ip) =
-                                            ip2_string(&*name.d.ip, &mut cert_ip_buf)
-                                        {
-                                            if hip == cert_ip {
-                                                return true;
-                                            }
+                            }
+                            boring::GEN_IPADD => {
+                                has_identifier_san = true;
+                                if let Some(hip) = host_ip {
+                                    if let Some(cert_ip) = ip2_string(&*name.d.ip, &mut cert_ip_buf)
+                                    {
+                                        if hip == cert_ip {
+                                            return true;
                                         }
                                     }
                                 }
-                                _ => {}
                             }
+                            _ => {}
                         }
                     }
                 }

src/boringssl_sys/boringssl.rs

@@ -280,10 +280,111 @@ pub(crate) struct OPENSSL_STACK {
     pub comp: OPENSSL_sk_cmp_func,
 }
 
+unsafe extern "C" {
+    fn GENERAL_NAME_free(name: *mut GENERAL_NAME);
+}
+
+/// Owns one `SSL_CTX` reference; `SSL_CTX_free`s it on drop. Construct from a
+/// pointer that already carries a +1 (`SSL_CTX_new`, `SSL_CTX_up_ref`).
+pub struct OwnedSslCtx(core::ptr::NonNull<SSL_CTX>);
+
+impl OwnedSslCtx {
+    /// Takes the +1 `raw` carries; `None` when `raw` is null.
+    ///
+    /// # Safety
+    /// `raw` must be null or carry a reference the caller is giving up.
+    pub unsafe fn from_raw(raw: *mut SSL_CTX) -> Option<Self> {
+        core::ptr::NonNull::new(raw).map(Self)
+    }
+
+    pub fn as_ptr(&self) -> *mut SSL_CTX {
+        self.0.as_ptr()
+    }
+
+    /// Transfers the reference back out; the caller must free it.
+    pub fn into_raw(self) -> *mut SSL_CTX {
+        core::mem::ManuallyDrop::new(self).0.as_ptr()
+    }
+}
+
+impl Drop for OwnedSslCtx {
+    fn drop(&mut self) {
+        // SAFETY: we own exactly one reference, released once.
+        unsafe { SSL_CTX_free(self.0.as_ptr()) }
+    }
+}
+
+/// Owns the `STACK_OF(GENERAL_NAME)` that `X509V3_EXT_d2i` returns for a
+/// subjectAltName extension. Frees every `GENERAL_NAME` and then the stack.
+pub struct GeneralNames(core::ptr::NonNull<struct_stack_st_GENERAL_NAME>);
+
+impl GeneralNames {
+    /// Takes ownership of a `STACK_OF(GENERAL_NAME)`; `None` when `raw` is null.
+    ///
+    /// # Safety
+    /// `raw` must be null or a stack the caller owns and does not free itself.
+    pub unsafe fn from_raw(raw: *mut c_void) -> Option<Self> {
+        core::ptr::NonNull::new(raw.cast::<struct_stack_st_GENERAL_NAME>()).map(Self)
+    }
+
+    pub fn len(&self) -> usize {
+        // SAFETY: we own a live stack; `sk_num` takes it as `const OPENSSL_STACK`.
+        unsafe { sk_num(self.0.as_ptr().cast::<OPENSSL_STACK>()) }
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    /// Borrows the `i`th entry; `None` past the end.
+    pub fn get(&self, i: usize) -> Option<&GENERAL_NAME> {
+        if i >= self.len() {
+            return None;
+        }
+        // SAFETY: `i` is in bounds and the stack outlives the borrow, which is
+        // tied to `&self`. BoringSSL owns the element until our `Drop`.
+        unsafe {
+            sk_value(self.0.as_ptr().cast::<OPENSSL_STACK>(), i)
+                .cast::<GENERAL_NAME>()
+                .as_ref()
+        }
+    }
+
+    pub fn iter(&self) -> impl Iterator<Item = &GENERAL_NAME> {
+        (0..self.len()).filter_map(|i| self.get(i))
+    }
+}
+
+impl Drop for GeneralNames {
+    fn drop(&mut self) {
+        // SAFETY: `sk_pop_free_ex` invokes the callback once per element, so it
+        // gets `GENERAL_NAME_free` (per element), not a stack free.
+        unsafe {
+            sk_pop_free_ex(
+                self.0.as_ptr().cast::<OPENSSL_STACK>(),
+                Some(call_general_name_free),
+                Some(core::mem::transmute::<
+                    unsafe extern "C" fn(*mut GENERAL_NAME),
+                    unsafe extern "C" fn(*mut c_void),
+                >(GENERAL_NAME_free)),
+            )
+        }
+    }
+}
+
+/// Restores the element type erased through `OPENSSL_sk_free_func`.
+unsafe extern "C" fn call_general_name_free(free_func: OPENSSL_sk_free_func, ptr: *mut c_void) {
+    // SAFETY: `free_func` is `GENERAL_NAME_free` erased in `Drop` above; both
+    // sides are `extern "C" fn(*mut _)`, so the round-trip is ABI-sound.
+    let f: unsafe extern "C" fn(*mut GENERAL_NAME) =
+        unsafe { core::mem::transmute(free_func.expect("non-null free_func")) };
+    // SAFETY: `ptr` is an element `sk_pop_free_ex` is draining from the stack.
+    unsafe { f(ptr.cast::<GENERAL_NAME>()) }
+}
+
 unsafe extern "C" {
     fn sk_num(sk: *const OPENSSL_STACK) -> usize;
     fn sk_value(sk: *const OPENSSL_STACK, i: usize) -> *mut c_void;
-    fn sk_free(sk: *mut OPENSSL_STACK);
     fn sk_pop_free_ex(
         sk: *mut OPENSSL_STACK,
         call_free_func: OPENSSL_sk_call_free_func,
@@ -439,9 +540,6 @@ unsafe extern "C" {
 // symbol — they bottom out on the untyped `sk_*` ABI above.
 // ═══════════════════════════════════════════════════════════════════════════
 
-/// Per-stack free callback type used by `sk_GENERAL_NAME_pop_free`.
-pub(crate) type sk_GENERAL_NAME_free_func = unsafe extern "C" fn(*mut struct_stack_st_GENERAL_NAME);
-
 #[inline]
 pub unsafe fn sk_X509_value(sk: *const struct_stack_st_X509, i: usize) -> *mut X509 {
     // SAFETY: Two independent type casts, not a const→mut provenance laundering:
@@ -452,73 +550,6 @@ pub unsafe fn sk_X509_value(sk: *const struct_stack_st_X509, i: usize) -> *mut X
     unsafe { sk_value(sk.cast::<OPENSSL_STACK>(), i).cast::<X509>() }
 }
 
-#[inline]
-pub unsafe fn sk_GENERAL_NAME_num(sk: *const struct_stack_st_GENERAL_NAME) -> usize {
-    // SAFETY: const→const cast between opaque aliases — `STACK_OF(GENERAL_NAME)`
-    // is the same C object as `OPENSSL_STACK`. Caller's `unsafe` contract
-    // guarantees `sk` is NULL or a live BoringSSL stack; `sk_num` accepts both.
-    unsafe { sk_num(sk.cast::<OPENSSL_STACK>()) }
-}
-
-#[inline]
-pub unsafe fn sk_GENERAL_NAME_value(
-    sk: *const struct_stack_st_GENERAL_NAME,
-    i: usize,
-) -> *mut GENERAL_NAME {
-    // SAFETY: `sk` cast is const→const between opaque stack types; the `*mut`
-    // return is narrowed from `sk_value`'s own `*mut c_void` result (C-heap
-    // provenance), not derived from `sk`. No const→mut on a single value.
-    unsafe { sk_value(sk.cast::<OPENSSL_STACK>(), i).cast::<GENERAL_NAME>() }
-}
-
-#[inline]
-pub unsafe extern "C" fn sk_GENERAL_NAME_free(sk: *mut struct_stack_st_GENERAL_NAME) {
-    // SAFETY: mut→mut cast between opaque aliases of the same allocation.
-    // Caller's `unsafe` contract guarantees `sk` is NULL or an owned
-    // BoringSSL stack; `sk_free` is documented to accept both.
-    unsafe { sk_free(sk.cast::<OPENSSL_STACK>()) }
-}
-
-unsafe extern "C" fn sk_GENERAL_NAME_call_free_func(
-    free_func: OPENSSL_sk_free_func,
-    ptr: *mut c_void,
-) {
-    // SAFETY: `free_func` was originally an `sk_GENERAL_NAME_free_func` erased
-    // through `OPENSSL_sk_free_func` by `sk_GENERAL_NAME_pop_free` below; both
-    // are `extern "C" fn(*mut _)` so the pointer round-trip is ABI-sound.
-    let f: sk_GENERAL_NAME_free_func = unsafe {
-        core::mem::transmute::<unsafe extern "C" fn(*mut c_void), sk_GENERAL_NAME_free_func>(
-            free_func.expect("non-null free_func"),
-        )
-    };
-    // SAFETY: `ptr` is an element handed to this trampoline by `sk_pop_free_ex`
-    // while draining the `STACK_OF(GENERAL_NAME)` passed in below; the cast
-    // restores the typed pointer `f` was declared to accept before erasure.
-    unsafe { f(ptr.cast::<struct_stack_st_GENERAL_NAME>()) }
-}
-
-#[inline]
-pub unsafe fn sk_GENERAL_NAME_pop_free(
-    sk: *mut struct_stack_st_GENERAL_NAME,
-    free_func: sk_GENERAL_NAME_free_func,
-) {
-    // SAFETY: `sk` cast is mut→mut between opaque aliases; caller guarantees it
-    // is NULL or an owned `STACK_OF(GENERAL_NAME)`. The transmute erases
-    // `free_func`'s typed arg to `*mut c_void` — both sides are
-    // `extern "C" fn(*mut _)` so the fn-pointer reinterpret is ABI-sound, and
-    // `sk_GENERAL_NAME_call_free_func` restores the type before invoking it.
-    unsafe {
-        sk_pop_free_ex(
-            sk.cast::<OPENSSL_STACK>(),
-            Some(sk_GENERAL_NAME_call_free_func),
-            Some(core::mem::transmute::<
-                sk_GENERAL_NAME_free_func,
-                unsafe extern "C" fn(*mut c_void),
-            >(free_func)),
-        )
-    }
-}
-
 // ═══════════════════════════════════════════════════════════════════════════
 // SSL / TLS — error codes, verify modes, shutdown flags, renegotiate modes
 // (`vendor/boringssl/include/openssl/ssl.h`)

src/http_jsc/websocket_client.rs

@@ -543,22 +543,14 @@ impl<const SSL: bool> WebSocket<SSL> {
         self.message_is_compressed.set(false);
     }
 
-    // takes a raw `*mut Self` instead of `&self` because
+    // Takes `ThisPtr<Self>` instead of `&self` because
     // `handle_without_deinit()` re-enters this very function on the same
     // allocation through its own raw back-pointer.
     //
     // There is no `socket` parameter: the dispatch thunk wraps the same
     // `us_socket_t*` that `adopt_group` stored into `self.tcp`, so the parse
     // loop reads `self.tcp` directly.
-    //
-    /// # Safety
-    /// `this_ptr` must point to a live `WebSocket<SSL>` allocated via
-    /// `heap::alloc` (see `init` / `init_with_tunnel`); no `&`/`&mut`
-    /// borrow of `*this_ptr` may be live across this call.
-    pub unsafe fn handle_data(this_ptr: *mut Self, data_: &[u8]) {
-        // SAFETY: caller contract — `this_ptr` is a live `heap::alloc` pointer
-        // with no outstanding `&`/`&mut` borrow (uWS dispatches from userdata).
-        let this = unsafe { ThisPtr::new(this_ptr) };
+    pub fn handle_data(this: ThisPtr<Self>, data_: &[u8]) {
         // after receiving close we should ignore the data
         if this.close_received.get() {
             return;
@@ -575,7 +567,7 @@ impl<const SSL: bool> WebSocket<SSL> {
             // We do not free the memory here since the lifetime is managed by the microtask queue (it should free when called from there)
             // SAFETY: `initial_handler` is valid (managed by microtask queue).
             // `handle_without_deinit` re-enters `Self::handle_data` via the
-            // `adopted` raw ptr (same `heap::alloc` provenance as `this_ptr`).
+            // `adopted` raw ptr (same `heap::alloc` provenance as `this`).
             unsafe { (*initial_handler.as_ptr()).handle_without_deinit() };
 
             // handle_without_deinit is supposed to clear the handler from WebSocket*
@@ -1708,8 +1700,9 @@ impl<const SSL: bool> WebSocket<SSL> {
     pub unsafe fn handle_tunnel_data(this_ptr: *mut Self, data: &[u8]) {
         // Process the decrypted data as if it came from the socket
         // has_tcp() now returns true for tunnel mode, so this will work correctly
-        // SAFETY: forwarded — see `handle_data`'s contract.
-        unsafe { Self::handle_data(this_ptr, data) };
+        // SAFETY: caller contract — `this_ptr` is a live `heap::alloc` pointer
+        // with no outstanding `&`/`&mut` borrow.
+        Self::handle_data(unsafe { ThisPtr::new(this_ptr) }, data);
     }
 
     /// Called by the WebSocketProxyTunnel when the underlying socket drains.
@@ -1984,10 +1977,10 @@ impl<const SSL: bool> InitialDataHandler<SSL> {
             unsafe { !(*ws_ptr).tcp.get().is_closed() || (*ws_ptr).proxy_tunnel.get().is_some() };
         // SAFETY: `ws_ptr` is live; raw read of a `Copy` field.
         if unsafe { (*ws_ptr).outgoing_websocket.get().is_some() } && is_connected {
-            // SAFETY: `ws_ptr` carries `heap::alloc` provenance; `handle_data`
-            // takes `*mut Self` and forms its own scoped `&mut` internally. No
+            // SAFETY: `ws_ptr` carries `heap::alloc` provenance and is live; no
             // borrow of `*ws_ptr` is live in this frame across the call.
-            unsafe { WebSocket::<SSL>::handle_data(ws_ptr, &self.slice) };
+            let ws = unsafe { ThisPtr::new(ws_ptr) };
+            WebSocket::<SSL>::handle_data(ws, &self.slice);
         }
     }