Skip to content

Commit c8bbb4f

Browse files
committed
fix: thread safe handle may no longer have an isolate pointer
There is no explicit lifetime synchronization path between the supervisor and isolate, so we cannot assume that `Isolate::request_interrupt` will always be scheduled. The `IsolateInterruptData` passed as an argument has always been passed to `Box::into_raw`, so if the isolation is ever destroyed, the step of converting it back to a `Box<T>` to free the memory is omitted. We should have looked at the result of the `request_interrupt` call and freed the passed arguments in some cases, but we didn't, which led to a memory leak. (cherry picked from commit 2790fa4)
1 parent 2def458 commit c8bbb4f

File tree

3 files changed

+28
-19
lines changed

3 files changed

+28
-19
lines changed

crates/base/src/rt_worker/supervisor/strategy_per_request.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,17 @@ pub async fn supervise(args: Arguments, oneshot: bool) -> (ShutdownReason, i64)
185185

186186
Some(reason) => {
187187
is_retired.raise();
188-
thread_safe_handle.request_interrupt(
189-
handle_interrupt,
190-
Box::into_raw(Box::new(IsolateInterruptData {
191-
should_terminate: true,
192-
isolate_memory_usage_tx: Some(isolate_memory_usage_tx),
193-
})) as *mut std::ffi::c_void,
194-
);
188+
189+
let data_ptr_mut = Box::into_raw(Box::new(IsolateInterruptData {
190+
should_terminate: true,
191+
isolate_memory_usage_tx: Some(isolate_memory_usage_tx),
192+
}));
193+
194+
if !thread_safe_handle
195+
.request_interrupt(handle_interrupt, data_ptr_mut as *mut std::ffi::c_void)
196+
{
197+
drop(unsafe { Box::from_raw(data_ptr_mut) });
198+
}
195199

196200
return (reason, cpu_usage_accumulated_ms);
197201
}

crates/base/src/rt_worker/supervisor/strategy_per_worker.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,16 @@ pub async fn supervise(args: Arguments) -> (ShutdownReason, i64) {
6262
let interrupt_fn = {
6363
let thread_safe_handle = thread_safe_handle.clone();
6464
move |should_terminate: bool| {
65-
let interrupt_data = IsolateInterruptData {
65+
let data_ptr_mut = Box::into_raw(Box::new(IsolateInterruptData {
6666
should_terminate,
6767
isolate_memory_usage_tx: Some(isolate_memory_usage_tx),
68-
};
68+
}));
6969

70-
thread_safe_handle.request_interrupt(
71-
handle_interrupt,
72-
Box::into_raw(Box::new(interrupt_data)) as *mut std::ffi::c_void,
73-
);
70+
if !thread_safe_handle
71+
.request_interrupt(handle_interrupt, data_ptr_mut as *mut std::ffi::c_void)
72+
{
73+
drop(unsafe { Box::from_raw(data_ptr_mut) });
74+
}
7475
}
7576
};
7677

crates/base/src/rt_worker/worker.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,16 +161,20 @@ impl Worker {
161161
rt::SUPERVISOR_RT
162162
.spawn(async move {
163163
token.inbound.cancelled().await;
164-
165164
is_termination_requested.raise();
166-
thread_safe_handle.request_interrupt(
167-
supervisor::handle_interrupt,
165+
166+
let data_ptr_mut =
168167
Box::into_raw(Box::new(supervisor::IsolateInterruptData {
169168
should_terminate: true,
170169
isolate_memory_usage_tx: None,
171-
}))
172-
as *mut std::ffi::c_void,
173-
);
170+
}));
171+
172+
if !thread_safe_handle.request_interrupt(
173+
supervisor::handle_interrupt,
174+
data_ptr_mut as *mut std::ffi::c_void,
175+
) {
176+
drop(unsafe { Box::from_raw(data_ptr_mut) });
177+
}
174178

175179
while !is_terminated.is_raised() {
176180
waker.wake();

0 commit comments

Comments
 (0)