Merge pull request supabase#260 from nyannyacha/fix-req-interrupt-leak

fix: thread safe handle may no longer have an isolate pointer
nyannyacha · Feb 6, 2024 · dcaaf35 · dcaaf35
2 parents f6d7faf + c8bbb4f
commit dcaaf35
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 19 deletions.
diff --git a/crates/base/src/rt_worker/supervisor/strategy_per_request.rs b/crates/base/src/rt_worker/supervisor/strategy_per_request.rs
@@ -185,13 +185,17 @@ pub async fn supervise(args: Arguments, oneshot: bool) -> (ShutdownReason, i64)
 
             Some(reason) => {
                 is_retired.raise();
-                thread_safe_handle.request_interrupt(
-                    handle_interrupt,
-                    Box::into_raw(Box::new(IsolateInterruptData {
-                        should_terminate: true,
-                        isolate_memory_usage_tx: Some(isolate_memory_usage_tx),
-                    })) as *mut std::ffi::c_void,
-                );
+
+                let data_ptr_mut = Box::into_raw(Box::new(IsolateInterruptData {
+                    should_terminate: true,
+                    isolate_memory_usage_tx: Some(isolate_memory_usage_tx),
+                }));
+
+                if !thread_safe_handle
+                    .request_interrupt(handle_interrupt, data_ptr_mut as *mut std::ffi::c_void)
+                {
+                    drop(unsafe { Box::from_raw(data_ptr_mut) });
+                }
 
                 return (reason, cpu_usage_accumulated_ms);
             }

diff --git a/crates/base/src/rt_worker/supervisor/strategy_per_worker.rs b/crates/base/src/rt_worker/supervisor/strategy_per_worker.rs
@@ -62,15 +62,16 @@ pub async fn supervise(args: Arguments) -> (ShutdownReason, i64) {
     let interrupt_fn = {
         let thread_safe_handle = thread_safe_handle.clone();
         move |should_terminate: bool| {
-            let interrupt_data = IsolateInterruptData {
+            let data_ptr_mut = Box::into_raw(Box::new(IsolateInterruptData {
                 should_terminate,
                 isolate_memory_usage_tx: Some(isolate_memory_usage_tx),
-            };
+            }));
 
-            thread_safe_handle.request_interrupt(
-                handle_interrupt,
-                Box::into_raw(Box::new(interrupt_data)) as *mut std::ffi::c_void,
-            );
+            if !thread_safe_handle
+                .request_interrupt(handle_interrupt, data_ptr_mut as *mut std::ffi::c_void)
+            {
+                drop(unsafe { Box::from_raw(data_ptr_mut) });
+            }
         }
     };
 

diff --git a/crates/base/src/rt_worker/worker.rs b/crates/base/src/rt_worker/worker.rs
@@ -161,16 +161,20 @@ impl Worker {
                             rt::SUPERVISOR_RT
                                 .spawn(async move {
                                     token.inbound.cancelled().await;
-
                                     is_termination_requested.raise();
-                                    thread_safe_handle.request_interrupt(
-                                        supervisor::handle_interrupt,
+
+                                    let data_ptr_mut =
                                         Box::into_raw(Box::new(supervisor::IsolateInterruptData {
                                             should_terminate: true,
                                             isolate_memory_usage_tx: None,
-                                        }))
-                                            as *mut std::ffi::c_void,
-                                    );
+                                        }));
+
+                                    if !thread_safe_handle.request_interrupt(
+                                        supervisor::handle_interrupt,
+                                        data_ptr_mut as *mut std::ffi::c_void,
+                                    ) {
+                                        drop(unsafe { Box::from_raw(data_ptr_mut) });
+                                    }
 
                                     while !is_terminated.is_raised() {
                                         waker.wake();