Double borrow panic lacks details (#82)

It's currently difficult to tell which script function is panicking when
a double mutable borrow occurs.

Author: TitanNano

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<Box<dyn GodotScriptObject>>,
     data_ptr: *mut Box<dyn GodotScriptObject>,
@@ -29,24 +33,27 @@ impl<Script: GodotScriptImpl> Debug for Context<'_, Script> {
 }
 
 impl<Script: GodotScriptImpl> 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<T: GodotScriptObject + 'static, Args, Return>(
         &mut self,
         self_ref: &mut T,
         scope: impl ReentrantScope<Script::ImplBase, Args, Return>,
     ) -> Return {
-        let known_ptr = unsafe {
-            let any = (*self.data_ptr).as_any_mut();
-
-            any.downcast_mut::<T>().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::<T>().unwrap() as *mut T;
 
         let self_ptr = self_ref as *mut _;
 
         if known_ptr != self_ptr {
             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,13 +65,20 @@ impl<Script: GodotScriptImpl> Context<'_, Script> {
     }
 }
 
+/// A generic script call context that is not tied to a specific script type.
 pub struct GenericContext<'a> {
     cell: *const GdCell<Box<dyn GodotScriptObject>>,
     data_ptr: *mut Box<dyn GodotScriptObject>,
     base: ScriptBaseMut<'a, RustScriptInstance>,
 }
 
 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<dyn GodotScriptObject>`] inside the [`GdCell`].
+    /// - both `cell` and `data_ptr` must out-live the `base`.
     pub(super) unsafe fn new(
         cell: *const GdCell<Box<dyn GodotScriptObject>>,
         data_ptr: *mut Box<dyn GodotScriptObject>,

rust-script/src/runtime/rust_script_instance.rs

@@ -131,13 +131,29 @@ impl ScriptInstance for RustScriptInstance {
 
     fn set_property(this: SiMut<Self>, 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<Variant> {
-        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)