Skip to content

Access to base pointer during initialization #1273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 225 additions & 6 deletions godot-core/src/obj/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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> {
Expand All @@ -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).
Expand All @@ -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.
Expand All @@ -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),
Expand All @@ -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 {
Expand All @@ -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> {
Expand Down
2 changes: 2 additions & 0 deletions godot-core/src/obj/call_deferred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions godot-core/src/obj/casts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/obj/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}

Expand Down
8 changes: 6 additions & 2 deletions godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading