Skip to content

Commit b91970c

Browse files
Remove opportunistic Arc<Mutex<>> extraction to ensure consistent FunctionRegistry sharing
- Simplified OutBHandler to consistently use Arc<Mutex<FunctionRegistry>> - Removed FunctionRegistryStorage enum and dual-path optimization - Eliminated handle_outb_impl_direct function - Streamlined outb_handler_wrapper to always use shared FunctionRegistry - Maintains necessary sharing between sandbox and OutBHandler components Co-authored-by: syntactically <168595099+syntactically@users.noreply.github.com>
1 parent e38f707 commit b91970c

File tree

1 file changed

+15
-112
lines changed
  • src/hyperlight_host/src/sandbox

1 file changed

+15
-112
lines changed

src/hyperlight_host/src/sandbox/outb.rs

Lines changed: 15 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -33,59 +33,33 @@ use crate::{new_error, HyperlightError, Result};
3333
/// A `OutBHandler` implementation that contains the required data directly
3434
///
3535
/// Note: This handler must live no longer than the `Sandbox` to which it belongs
36-
/// Note: To avoid double Arc<Mutex<>> wrapping, we store FunctionRegistry directly
37-
/// when possible, falling back to Arc<Mutex<>> only when necessary for sharing.
3836
pub(crate) struct OutBHandler {
3937
mem_mgr_wrapper: MemMgrWrapper<HostSharedMemory>,
40-
host_funcs: FunctionRegistryStorage,
41-
}
42-
43-
/// Storage for FunctionRegistry that avoids double Arc<Mutex<>> wrapping when possible
44-
enum FunctionRegistryStorage {
45-
/// Direct ownership when FunctionRegistry doesn't need to be shared
46-
Owned(FunctionRegistry),
47-
/// Shared ownership when FunctionRegistry needs to be shared with other components
48-
Shared(Arc<Mutex<FunctionRegistry>>),
38+
host_funcs: Arc<Mutex<FunctionRegistry>>,
4939
}
5040

5141
impl OutBHandler {
52-
/// Create a new OutBHandler with owned FunctionRegistry to avoid double Arc<Mutex<>> wrapping
53-
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
54-
pub fn new_with_owned_registry(
55-
mem_mgr_wrapper: MemMgrWrapper<HostSharedMemory>,
56-
host_funcs: FunctionRegistry,
57-
) -> Self {
58-
Self {
59-
mem_mgr_wrapper,
60-
host_funcs: FunctionRegistryStorage::Owned(host_funcs),
61-
}
62-
}
63-
64-
/// Create a new OutBHandler with shared FunctionRegistry (fallback for compatibility)
42+
/// Create a new OutBHandler
6543
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
66-
pub fn new_with_shared_registry(
44+
pub fn new(
6745
mem_mgr_wrapper: MemMgrWrapper<HostSharedMemory>,
68-
host_funcs_wrapper: Arc<Mutex<FunctionRegistry>>,
46+
host_funcs: Arc<Mutex<FunctionRegistry>>,
6947
) -> Self {
7048
Self {
7149
mem_mgr_wrapper,
72-
host_funcs: FunctionRegistryStorage::Shared(host_funcs_wrapper),
50+
host_funcs,
7351
}
7452
}
7553

7654
/// Function that gets called when an outb operation has occurred.
7755
#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
7856
pub fn call(&mut self, port: u16, payload: u32) -> Result<()> {
79-
match &mut self.host_funcs {
80-
FunctionRegistryStorage::Owned(ref registry) => {
81-
// Direct access - no extra locking needed
82-
handle_outb_impl_direct(&mut self.mem_mgr_wrapper, registry, port, payload)
83-
}
84-
FunctionRegistryStorage::Shared(arc_mutex) => {
85-
// Shared access - requires locking (original behavior)
86-
handle_outb_impl(&mut self.mem_mgr_wrapper, arc_mutex.clone(), port, payload)
87-
}
88-
}
57+
handle_outb_impl(
58+
&mut self.mem_mgr_wrapper,
59+
self.host_funcs.clone(),
60+
port,
61+
payload,
62+
)
8963
}
9064
}
9165

@@ -217,10 +191,6 @@ pub(crate) fn handle_outb_impl(
217191
let name = call.function_name.clone();
218192
let args: Vec<ParameterValue> = call.parameters.unwrap_or(vec![]);
219193

220-
// Lock the FunctionRegistry for the minimal time needed to call the function
221-
// Note: This creates a brief double-lock situation (OutBHandler + FunctionRegistry)
222-
// but it's unavoidable given the current architecture where FunctionRegistry
223-
// needs to be shared between OutBHandler and other components
224194
let res = host_funcs
225195
.lock()
226196
.map_err(|e| {
@@ -254,82 +224,15 @@ pub(crate) fn handle_outb_impl(
254224
}
255225
}
256226

257-
/// Handles OutB operations from the guest (direct access version - no double locking).
258-
#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
259-
pub(crate) fn handle_outb_impl_direct(
260-
mem_mgr: &mut MemMgrWrapper<HostSharedMemory>,
261-
host_funcs: &FunctionRegistry,
262-
port: u16,
263-
data: u32,
264-
) -> Result<()> {
265-
match port.try_into()? {
266-
OutBAction::Log => outb_log(mem_mgr.as_mut()),
267-
OutBAction::CallFunction => {
268-
let call = mem_mgr.as_mut().get_host_function_call()?; // pop output buffer
269-
let name = call.function_name.clone();
270-
let args: Vec<ParameterValue> = call.parameters.unwrap_or(vec![]);
271-
272-
// Direct call - no locking needed since we have direct access to FunctionRegistry
273-
let res = host_funcs.call_host_function(&name, args)?;
274-
275-
mem_mgr
276-
.as_mut()
277-
.write_response_from_host_method_call(&res)?; // push input buffers
278-
279-
Ok(())
280-
}
281-
OutBAction::Abort => outb_abort(mem_mgr, data),
282-
OutBAction::DebugPrint => {
283-
let ch: char = match char::from_u32(data) {
284-
Some(c) => c,
285-
None => {
286-
return Err(new_error!("Invalid character for logging: {}", data));
287-
}
288-
};
289-
290-
eprint!("{}", ch);
291-
Ok(())
292-
}
293-
}
294-
}
295-
296-
/// Given a `MemMgrWrapper` and ` HostFuncsWrapper` -- both passed by _value_
297-
/// -- return an `Arc<Mutex<OutBHandler>>` wrapping the core OUTB handler logic.
298-
///
299-
/// This function attempts to optimize away double Arc<Mutex<>> wrapping by checking
300-
/// if the FunctionRegistry can be moved out of its Arc<Mutex<>> (i.e., when it's
301-
/// no longer shared with other components).
227+
/// Given a `MemMgrWrapper` and `Arc<Mutex<FunctionRegistry>>` -- both passed by _value_
228+
/// -- return an `Arc<Mutex<OutBHandler>>` wrapping the core OUTB handler logic.
302229
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
303230
pub(crate) fn outb_handler_wrapper(
304231
mem_mgr_wrapper: MemMgrWrapper<HostSharedMemory>,
305232
host_funcs_wrapper: Arc<Mutex<FunctionRegistry>>,
306233
) -> Arc<Mutex<OutBHandler>> {
307-
// Try to extract the FunctionRegistry from Arc<Mutex<>> to avoid double wrapping
308-
match Arc::try_unwrap(host_funcs_wrapper) {
309-
Ok(mutex) => {
310-
// Successfully extracted Arc, now extract from Mutex
311-
match mutex.into_inner() {
312-
Ok(function_registry) => {
313-
// Success! We can use direct ownership
314-
let outb_hdl =
315-
OutBHandler::new_with_owned_registry(mem_mgr_wrapper, function_registry);
316-
Arc::new(Mutex::new(outb_hdl))
317-
}
318-
Err(poisoned) => {
319-
// Mutex was poisoned, fall back to shared access
320-
let function_registry = poisoned.into_inner();
321-
let outb_hdl =
322-
OutBHandler::new_with_owned_registry(mem_mgr_wrapper, function_registry);
323-
Arc::new(Mutex::new(outb_hdl))
324-
}
325-
}
326-
}
327-
Err(arc_mutex) => {
328-
// Arc has multiple references, fall back to shared access
329-
let outb_hdl = OutBHandler::new_with_shared_registry(mem_mgr_wrapper, arc_mutex);
330-
Arc::new(Mutex::new(outb_hdl))
331-
}
332-
}
234+
let outb_hdl = OutBHandler::new(mem_mgr_wrapper, host_funcs_wrapper);
235+
Arc::new(Mutex::new(outb_hdl))
333236
}
334237

335238
#[cfg(test)]

0 commit comments

Comments
 (0)