Merge pull request #1273 from godot-rust/feature/base-during-init

Access to base pointer during initialization

Author: Bromeon

check.sh

@@ -35,7 +35,7 @@ Commands:
 
 Options:
     -h, --help               print this help text
-    --double                 run check with double-precision
+    --double                 run check with double-precision (implies `api-custom` feature)
     -f, --filter <arg>       only run integration tests which contain any of the
                              args (comma-separated). requires itest.
     -a, --api-version <ver>  specify the Godot API version to use (e.g. 4.3, 4.3.1).
@@ -209,7 +209,7 @@ while [[ $# -gt 0 ]]; do
             extraCargoArgs+=("--features" "serde")
             ;;
         --double)
-            extraCargoArgs+=("--features" "godot/double-precision")
+            extraCargoArgs+=("--features" "godot/double-precision,godot/api-custom")
             ;;
         fmt | test | itest | clippy | klippy | doc | dok)
             cmds+=("$arg")

godot-core/src/obj/base.rs

@@ -5,12 +5,55 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+#[cfg(debug_assertions)]
+use std::cell::Cell;
+use std::cell::RefCell;
+use std::collections::hash_map::Entry;
+use std::collections::HashMap;
 use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
 use std::mem::ManuallyDrop;
+use std::rc::Rc;
 
-use crate::obj::{Gd, GodotClass};
+use crate::builtin::{Callable, Variant};
+use crate::obj::{bounds, Gd, GodotClass, InstanceId};
 use crate::{classes, sys};
 
+thread_local! {
+    /// Extra strong references for each instance ID, needed for [`Base::to_init_gd()`].
+    ///
+    /// At the moment, all Godot objects must be accessed from the main thread, because their deferred destruction (`Drop`) runs on the
+    /// main thread, too. This may be relaxed in the future, and a `sys::Global` could be used instead of a `thread_local!`.
+    static PENDING_STRONG_REFS: RefCell<HashMap<InstanceId, Gd<classes::RefCounted>>> = RefCell::new(HashMap::new());
+}
+
+/// Represents the initialization state of a `Base<T>` object.
+#[cfg(debug_assertions)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+enum InitState {
+    /// Object is being constructed (inside `I*::init()` or `Gd::from_init_fn()`).
+    ObjectConstructing,
+    /// Object construction is complete.
+    ObjectInitialized,
+    /// `ScriptInstance` context - always considered initialized (bypasses lifecycle checks).
+    Script,
+}
+
+#[cfg(debug_assertions)]
+macro_rules! base_from_obj {
+    ($obj:expr, $state:expr) => {
+        Base::from_obj($obj, $state)
+    };
+}
+
+#[cfg(not(debug_assertions))]
+macro_rules! base_from_obj {
+    ($obj:expr, $state:expr) => {
+        Base::from_obj($obj)
+    };
+}
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+
 /// Restricted version of `Gd`, to hold the base instance inside a user's `GodotClass`.
 ///
 /// Behaves similarly to [`Gd`][crate::obj::Gd], but is more constrained. Cannot be constructed by the user.
@@ -35,6 +78,12 @@ pub struct Base<T: GodotClass> {
     // 1.   Gd<T>  -- triggers InstanceStorage destruction
     // 2.
     obj: ManuallyDrop<Gd<T>>,
+
+    /// Tracks the initialization state of this `Base<T>` in Debug mode.
+    ///
+    /// Rc allows to "copy-construct" the base from an existing one, while still affecting the user-instance through the original `Base<T>`.
+    #[cfg(debug_assertions)]
+    init_state: Rc<Cell<InitState>>,
 }
 
 impl<T: GodotClass> Base<T> {
@@ -47,7 +96,14 @@ impl<T: GodotClass> Base<T> {
     /// If `base` is destroyed while the returned `Base<T>` is in use, that constitutes a logic error, not a safety issue.
     pub(crate) unsafe fn from_base(base: &Base<T>) -> Base<T> {
         debug_assert!(base.obj.is_instance_valid());
-        Base::from_obj(Gd::from_obj_sys_weak(base.obj.obj_sys()))
+
+        let obj = Gd::from_obj_sys_weak(base.obj.obj_sys());
+
+        Self {
+            obj: ManuallyDrop::new(obj),
+            #[cfg(debug_assertions)]
+            init_state: Rc::clone(&base.init_state),
+        }
     }
 
     /// Create base from existing object (used in script instances).
@@ -57,9 +113,11 @@ impl<T: GodotClass> Base<T> {
     /// # Safety
     /// `gd` must be alive at the time of invocation. If it is destroyed while the returned `Base<T>` is in use, that constitutes a logic
     /// error, not a safety issue.
-    pub(crate) unsafe fn from_gd(gd: &Gd<T>) -> Self {
+    pub(crate) unsafe fn from_script_gd(gd: &Gd<T>) -> Self {
         debug_assert!(gd.is_instance_valid());
-        Base::from_obj(Gd::from_obj_sys_weak(gd.obj_sys()))
+
+        let obj = Gd::from_obj_sys_weak(gd.obj_sys());
+        base_from_obj!(obj, InitState::Script)
     }
 
     /// Create new base from raw Godot object.
@@ -72,17 +130,26 @@ impl<T: GodotClass> Base<T> {
     pub(crate) unsafe fn from_sys(base_ptr: sys::GDExtensionObjectPtr) -> Self {
         assert!(!base_ptr.is_null(), "instance base is null pointer");
 
-        // Initialize only as weak pointer (don't increment reference count)
+        // Initialize only as weak pointer (don't increment reference count).
         let obj = Gd::from_obj_sys_weak(base_ptr);
 
         // This obj does not contribute to the strong count, otherwise we create a reference cycle:
         // 1. RefCounted (dropped in GDScript)
         // 2. holds user T (via extension instance and storage)
         // 3. holds Base<T> RefCounted (last ref, dropped in T destructor, but T is never destroyed because this ref keeps storage alive)
         // Note that if late-init never happened on self, we have the same behavior (still a raw pointer instead of weak Gd)
-        Base::from_obj(obj)
+        base_from_obj!(obj, InitState::ObjectConstructing)
     }
 
+    #[cfg(debug_assertions)]
+    fn from_obj(obj: Gd<T>, init_state: InitState) -> Self {
+        Self {
+            obj: ManuallyDrop::new(obj),
+            init_state: Rc::new(Cell::new(init_state)),
+        }
+    }
+
+    #[cfg(not(debug_assertions))]
     fn from_obj(obj: Gd<T>) -> Self {
         Self {
             obj: ManuallyDrop::new(obj),
@@ -94,10 +161,132 @@ impl<T: GodotClass> Base<T> {
     /// Using this method to call methods on the base field of a Rust object is discouraged, instead use the
     /// methods from [`WithBaseField`](super::WithBaseField) when possible.
     #[doc(hidden)]
+    #[deprecated = "Private API. Use `Base::to_init_gd()` or `WithBaseField::to_gd()` instead."] // TODO(v0.4): remove.
     pub fn to_gd(&self) -> Gd<T> {
         (*self.obj).clone()
     }
 
+    /// Returns a [`Gd`] referencing the base object, for exclusive use during object initialization.
+    ///
+    /// Can be used during an initialization function [`I*::init()`][crate::classes::IObject::init] or [`Gd::from_init_fn()`].
+    ///
+    /// The base pointer is only pointing to a base object; you cannot yet downcast it to the object being constructed.
+    /// The instance ID is the same as the one the in-construction object will have.
+    ///
+    /// # Lifecycle for ref-counted classes
+    /// If `T: Inherits<RefCounted>`, then the ref-counted object is not yet fully-initialized at the time of the `init` function running.
+    /// Accessing the base object without further measures would be dangerous. Here, godot-rust employs a workaround: the `Base` object (which
+    /// holds a weak pointer to the actual instance) is temporarily upgraded to a strong pointer, preventing use-after-free.
+    ///
+    /// This additional reference is automatically dropped at an implementation-defined point in time (which may change, and technically delay
+    /// destruction of your object as soon as you use `Base::to_init_gd()`). Right now, this refcount-decrement is deferred to the next frame.
+    ///
+    /// For now, ref-counted bases can only use `to_init_gd()` on the main thread.
+    ///
+    /// # Panics (Debug)
+    /// If called outside an initialization function, or for ref-counted objects on a non-main thread.
+    #[cfg(since_api = "4.2")]
+    pub fn to_init_gd(&self) -> Gd<T> {
+        #[cfg(debug_assertions)] // debug_assert! still checks existence of symbols.
+        assert!(
+            self.is_initializing(),
+            "Base::to_init_gd() can only be called during object initialization, inside I*::init() or Gd::from_init_fn()"
+        );
+
+        // For manually-managed objects, regular clone is fine.
+        // Only static type matters, because this happens immediately after initialization, so T is both static and dynamic type.
+        if !<T::Memory as bounds::Memory>::IS_REF_COUNTED {
+            return Gd::clone(&self.obj);
+        }
+
+        debug_assert!(
+            sys::is_main_thread(),
+            "Base::to_init_gd() can only be called on the main thread for ref-counted objects (for now)"
+        );
+
+        // First time handing out a Gd<T>, we need to take measures to temporarily upgrade the Base's weak pointer to a strong one.
+        // During the initialization phase (derived object being constructed), increment refcount by 1.
+        let instance_id = self.obj.instance_id();
+        PENDING_STRONG_REFS.with(|refs| {
+            let mut pending_refs = refs.borrow_mut();
+            if let Entry::Vacant(e) = pending_refs.entry(instance_id) {
+                let strong_ref: Gd<T> = unsafe { Gd::from_obj_sys(self.obj.obj_sys()) };
+
+                // We know that T: Inherits<RefCounted> due to check above, but don't it as a static bound for Gd::upcast().
+                // Thus fall back to low-level FFI cast on RawGd.
+                let strong_ref_raw = strong_ref.raw;
+                let raw = strong_ref_raw
+                    .ffi_cast::<classes::RefCounted>()
+                    .expect("Base must be RefCounted")
+                    .into_dest(strong_ref_raw);
+
+                e.insert(Gd { raw });
+            }
+        });
+
+        let name = format!("Base<{}> deferred unref", T::class_name());
+        let callable = Callable::from_once_fn(&name, move |_args| {
+            Self::drop_strong_ref(instance_id);
+            Ok(Variant::nil())
+        });
+
+        // Use Callable::call_deferred() instead of Gd::apply_deferred(). The latter implicitly borrows &mut self,
+        // causing a "destroyed while bind was active" panic.
+        callable.call_deferred(&[]);
+
+        (*self.obj).clone()
+    }
+
+    /// Drops any extra strong references, possibly causing object destruction.
+    fn drop_strong_ref(instance_id: InstanceId) {
+        PENDING_STRONG_REFS.with(|refs| {
+            let mut pending_refs = refs.borrow_mut();
+            let strong_ref = pending_refs.remove(&instance_id);
+            assert!(
+                strong_ref.is_some(),
+                "Base unexpectedly had its strong ref rug-pulled"
+            );
+
+            // Triggers RawGd::drop() -> dec-ref -> possibly object destruction.
+        });
+    }
+
+    /// Finalizes the initialization of this `Base<T>` and returns whether
+    pub(crate) fn mark_initialized(&mut self) {
+        #[cfg(debug_assertions)]
+        {
+            assert_eq!(
+                self.init_state.get(),
+                InitState::ObjectConstructing,
+                "Base<T> is already initialized, or holds a script instance"
+            );
+
+            self.init_state.set(InitState::ObjectInitialized);
+        }
+
+        // May return whether there is a "surplus" strong ref in the future, as self.extra_strong_ref.borrow().is_some().
+    }
+
+    /// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed.
+    #[doc(hidden)]
+    pub fn __fully_constructed_gd(&self) -> Gd<T> {
+        #[cfg(debug_assertions)] // debug_assert! still checks existence of symbols.
+        assert!(
+            !self.is_initializing(),
+            "WithBaseField::to_gd(), base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()"
+        );
+
+        (*self.obj).clone()
+    }
+
+    /// Returns a [`Gd`] referencing the base object, for use in script contexts only.
+    #[doc(hidden)]
+    pub fn __script_gd(&self) -> Gd<T> {
+        // Used internally by `SiMut::base()` and `SiMut::base_mut()` for script re-entrancy.
+        // Could maybe add debug validation to ensure script context in the future.
+        (*self.obj).clone()
+    }
+
     // Currently only used in outbound virtual calls (for scripts); search for: base_field(self).obj_sys().
     #[doc(hidden)]
     pub fn obj_sys(&self) -> sys::GDExtensionObjectPtr {
@@ -109,6 +298,36 @@ impl<T: GodotClass> Base<T> {
     pub(crate) fn debug_instance_id(&self) -> crate::obj::InstanceId {
         self.obj.instance_id()
     }
+
+    /// Returns a [`Gd`] referencing the base object, for use in script contexts only.
+    pub(crate) fn to_script_gd(&self) -> Gd<T> {
+        #[cfg(debug_assertions)]
+        assert_eq!(
+            self.init_state.get(),
+            InitState::Script,
+            "to_script_gd() can only be called on script-context Base objects"
+        );
+
+        (*self.obj).clone()
+    }
+
+    /// Returns `true` if this `Base<T>` is currently in the initializing state.
+    #[cfg(debug_assertions)]
+    fn is_initializing(&self) -> bool {
+        self.init_state.get() == InitState::ObjectConstructing
+    }
+
+    /// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed.
+    #[doc(hidden)]
+    pub fn __constructed_gd(&self) -> Gd<T> {
+        #[cfg(debug_assertions)] // debug_assert! still checks existence of symbols.
+        assert!(
+            !self.is_initializing(),
+            "WithBaseField::to_gd(), base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()"
+        );
+
+        (*self.obj).clone()
+    }
 }
 
 impl<T: GodotClass> Debug for Base<T> {

godot-core/src/obj/call_deferred.rs

@@ -20,6 +20,8 @@ use crate::registry::signal::ToSignalObj;
 #[cfg(before_api = "4.2")]
 pub trait WithDeferredCall<T: GodotClass> {}
 
+// TODO(v0.4): seal this and similar traits.
+
 /// Enables `Gd::apply_deferred()` for type-safe deferred calls.
 ///
 /// The trait is automatically available for all engine-defined Godot classes and user classes containing a `Base<T>` field.

godot-core/src/obj/casts.rs

@@ -48,6 +48,7 @@ impl<T: GodotClass, U: GodotClass> CastSuccess<T, U> {
     }
 
     /// Access shared reference to destination, without consuming object.
+    #[cfg(debug_assertions)]
     pub fn as_dest_ref(&self) -> &RawGd<U> {
         self.check_validity();
         &self.dest

godot-core/src/obj/script.rs

@@ -319,7 +319,7 @@ pub unsafe fn create_script_instance<T: ScriptInstance>(
         method_lists: BoundedPtrList::new(),
         // SAFETY: The script instance is always freed before the base object is destroyed. The weak reference should therefore never be
         // accessed after it has been freed.
-        base: unsafe { Base::from_gd(&for_object) },
+        base: unsafe { Base::from_script_gd(&for_object) },
     };
 
     let data_ptr = Box::into_raw(Box::new(data));
@@ -454,7 +454,7 @@ impl<'a, T: ScriptInstance> SiMut<'a, T> {
     /// }
     /// ```
     pub fn base(&self) -> ScriptBaseRef<'_, T> {
-        ScriptBaseRef::new(self.base_ref.to_gd(), self.mut_ref)
+        ScriptBaseRef::new(self.base_ref.to_script_gd(), self.mut_ref)
     }
 
     /// Returns a mutable reference suitable for calling engine methods on this object.
@@ -518,7 +518,7 @@ impl<'a, T: ScriptInstance> SiMut<'a, T> {
     pub fn base_mut(&mut self) -> ScriptBaseMut<'_, T> {
         let guard = self.cell.make_inaccessible(self.mut_ref).unwrap();
 
-        ScriptBaseMut::new(self.base_ref.to_gd(), guard)
+        ScriptBaseMut::new(self.base_ref.to_script_gd(), guard)
     }
 }
 

godot-core/src/obj/traits.rs

@@ -355,9 +355,13 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
     ///
     /// This is intended to be stored or passed to engine methods. You cannot call `bind()` or `bind_mut()` on it, while the method
     /// calling `to_gd()` is still running; that would lead to a double borrow panic.
+    ///
+    /// # Panics
+    /// If called during initialization (the `init()` function or `Gd::from_init_fn()`). Use [`Base::to_init_gd()`] instead.
     fn to_gd(&self) -> Gd<Self>;
 
     /// Returns a reference to the `Base` stored by this object.
+    #[doc(hidden)]
     fn base_field(&self) -> &Base<Self::Base>;
 
     /// Returns a shared reference suitable for calling engine methods on this object.
@@ -419,7 +423,7 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
     ///
     /// For this, use [`base_mut()`](WithBaseField::base_mut()) instead.
     fn base(&self) -> BaseRef<'_, Self> {
-        let gd = self.base_field().to_gd();
+        let gd = self.base_field().__constructed_gd();
 
         BaseRef::new(gd, self)
     }
@@ -490,7 +494,7 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
     /// ```
     #[allow(clippy::let_unit_value)]
     fn base_mut(&mut self) -> BaseMut<'_, Self> {
-        let base_gd = self.base_field().to_gd();
+        let base_gd = self.base_field().__constructed_gd();
 
         let gd = self.to_gd();
         // SAFETY: