From c30c954dfcd52d842dfedc70cca84b9599bd8980 Mon Sep 17 00:00:00 2001 From: Jovan Gerodetti Date: Mon, 10 Nov 2025 19:03:29 +0100 Subject: [PATCH] Double borrow panic lacks details --- rust-script/src/runtime/call_context.rs | 24 +++++++++--- .../src/runtime/rust_script_instance.rs | 37 +++++++++++++++++-- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/rust-script/src/runtime/call_context.rs b/rust-script/src/runtime/call_context.rs index a7df708..30239bf 100644 --- a/rust-script/src/runtime/call_context.rs +++ b/rust-script/src/runtime/call_context.rs @@ -15,6 +15,10 @@ use crate::interface::GodotScriptImpl; use super::rust_script_instance::{GodotScriptObject, RustScriptInstance}; +/// A call context for a script method call. +/// +/// The call context can be used to perform re-entrant calls into engine APIs. Its lifetime is constrained to the duration of the current +/// function. pub struct Context<'a, Script: GodotScriptImpl + ?Sized> { cell: *const GdCell>, data_ptr: *mut Box, @@ -29,16 +33,17 @@ impl Debug for Context<'_, Script> { } impl Context<'_, Script> { + /// Create a scope in which the current mutable ref to [`Self`] is released. + /// + /// A re-entrant scope allows to use engine APIs that call back into the current script. pub fn reentrant_scope( &mut self, self_ref: &mut T, scope: impl ReentrantScope, ) -> Return { - let known_ptr = unsafe { - let any = (*self.data_ptr).as_any_mut(); - - any.downcast_mut::().unwrap() as *mut T - }; + // SAFETY: the caller guaranteed that the data_ptr is valid for the lifetime of `Self`. + let known_box_ptr = unsafe { &mut *self.data_ptr }; + let known_ptr = known_box_ptr.as_any_mut().downcast_mut::().unwrap() as *mut T; let self_ptr = self_ref as *mut _; @@ -46,7 +51,9 @@ impl Context<'_, Script> { panic!("unable to create reentrant scope with unrelated self reference!"); } + // SAFETY: the caller guaranteed that the data_ptr is valid for the lifetime of `Self`. let current_ref = unsafe { &mut *self.data_ptr }; + // SAFETY: the caller guaranteed that the cell is valid for the lifetime of `Self`. let cell = unsafe { &*self.cell }; let guard = cell.make_inaccessible(current_ref).unwrap(); @@ -58,6 +65,7 @@ impl Context<'_, Script> { } } +/// A generic script call context that is not tied to a specific script type. pub struct GenericContext<'a> { cell: *const GdCell>, data_ptr: *mut Box, @@ -65,6 +73,12 @@ pub struct GenericContext<'a> { } impl<'a> GenericContext<'a> { + /// Create a new script call context. + /// + /// # Safety + /// - cell must be a valid pointer to a [`GdCell`] & not null. + /// - data_ptr must be a valid pointer to the [`Box`] inside the [`GdCell`]. + /// - both `cell` and `data_ptr` must out-live the `base`. pub(super) unsafe fn new( cell: *const GdCell>, data_ptr: *mut Box, diff --git a/rust-script/src/runtime/rust_script_instance.rs b/rust-script/src/runtime/rust_script_instance.rs index 8c4df08..f1aba19 100644 --- a/rust-script/src/runtime/rust_script_instance.rs +++ b/rust-script/src/runtime/rust_script_instance.rs @@ -131,13 +131,29 @@ impl ScriptInstance for RustScriptInstance { fn set_property(this: SiMut, name: StringName, value: &Variant) -> bool { let cell_ref = &this.data; - let mut mut_data = cell_ref.borrow_mut().unwrap(); + let mut mut_data = match cell_ref.borrow_mut() { + Ok(guard) => guard, + Err(err) => { + panic!( + "Error while writing to script property {}::{name}: {err}", + this.script.bind().get_class_name() + ); + } + }; mut_data.set(name, value.to_owned()) } fn get_property(&self, name: StringName) -> Option { - let guard = self.data.borrow().unwrap(); + let guard = match self.data.borrow() { + Ok(guard) => guard, + Err(err) => { + panic!( + "Error while reading script property {}::{name}: {err}", + self.script.bind().get_class_name() + ); + } + }; guard.get(name) } @@ -159,10 +175,25 @@ impl ScriptInstance for RustScriptInstance { let base = this.base_mut(); - let mut data_guard = unsafe { &*cell }.borrow_mut().unwrap(); + // SAFETY: cell pointer was just created and is valid. It will not out-live the current function. + let mut data_guard = match unsafe { &*cell }.borrow_mut() { + Ok(guard) => guard, + Err(err) => { + drop(base); + + panic!( + "Error while calling script function {}::{}: {}", + this.script.bind().get_class_name(), + method, + err + ); + } + }; let data = data_guard.deref_mut(); let data_ptr = data as *mut _; + // SAFETY: cell & data_ptr are valid for the duration of the call. The context can not out-live the current function as it's tied + // to the lifetime of the base ref. let context = unsafe { GenericContext::new(cell, data_ptr, base) }; data.call(method, args, context)