Skip to content
Merged
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
6 changes: 4 additions & 2 deletions godot-codegen/src/generator/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,10 @@ fn make_constructor_and_default(class: &Class, ctx: &Context) -> Construction {
quote! {
impl crate::obj::Singleton for #class_name {
fn singleton() -> crate::obj::Gd<Self> {
// SAFETY: Class name matches type T, per code generator.
unsafe { crate::classes::singleton_unchecked_type(&#godot_class_stringname) }
static CACHE: crate::classes::SingletonCache = crate::classes::SingletonCache::new();

// SAFETY: Class name matches type T, per code generator. Built-in engine singleton -> safe to cache.
unsafe { crate::classes::cached_singleton::<Self>(&CACHE, || #godot_class_stringname) }
}
}
}
Expand Down
97 changes: 96 additions & 1 deletion godot-core/src/classes/class_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
//! Runtime checks and inspection of Godot classes.

use std::fmt::Write;
use std::sync::atomic::{AtomicPtr, AtomicU64};

use crate::builtin::{GString, StringName, Variant};
use crate::obj::{Bounds, EngineBitfield, Gd, GodotClass, InstanceId, RawGd, bounds};
use crate::sys;
use crate::{init, sys};

#[cfg(safeguards_strict)]
mod strict {
Expand Down Expand Up @@ -274,6 +275,100 @@ where
}
}

/// Per-singleton cache for built-in engine singletons.
///
/// `level == None` means "not cached"; otherwise it holds the level at which `ptr` was fetched and `generation` the deinit generation then. `ptr` is
/// trusted only while the current level still covers `level` and no full deinit happened since (see [`init::singleton_cache_generation`]).
///
/// Ordering: the slow path writes `ptr`/`generation` `Relaxed`, then stores `level` last with `Release`. The fast path reads `level` first with
/// `Acquire`; that pairs with the store, making the earlier `ptr`/`generation` writes visible, so they can be read `Relaxed`.
pub(crate) struct SingletonCache {
ptr: AtomicPtr<std::ffi::c_void>,
generation: AtomicU64,
level: sys::AtomicEnum<Option<init::InitLevel>>,
}

impl SingletonCache {
// Not Default::default() because of const.
pub const fn new() -> Self {
Self {
ptr: AtomicPtr::new(std::ptr::null_mut()),
generation: AtomicU64::new(0),
level: sys::AtomicEnum::default(), // None.
}
}
}

/// Cached variant of [`singleton_unchecked_type`], for built-in engine singletons.
///
/// Engine singletons have a stable pointer for as long as their init level is loaded, so the `global_get_singleton()` lookup (plus `StringName`
/// construction) is just overhead. This caches the ptr per singleton type, gated on the init level to structurally prevent stale-pointer use.
///
/// # Safety
/// Same as [`singleton_unchecked_type`]: `make_class_name` must yield the class name matching type `T`, and this must only be used for engine
/// singletons (stable pointer for their level's lifetime).
pub(crate) unsafe fn cached_singleton<T>(
cache: &SingletonCache,
make_class_name: impl FnOnce() -> StringName,
) -> Gd<T>
where
T: GodotClass,
{
use std::sync::atomic::Ordering;

let current = init::current_init_level();
let generation = init::singleton_cache_generation();

// Fast path: trust cache if current level still covers cached level and no full deinit since. Read `level` first (`Acquire`), then
// `ptr`/`generation` `Relaxed` (see SingletonCache docs).
if let Some(cached_level) = cache.level.load()
&& current.is_some_and(|current| current >= cached_level)
&& cache.generation.load(Ordering::Relaxed) == generation
{
let ptr = cache.ptr.load(Ordering::Relaxed);
// SAFETY: level + generation guard guarantee singleton still alive; from_obj_sys handles ref-count.
return unsafe { Gd::<T>::from_obj_sys(ptr.cast()) };
}

// Slow path. Missing FFI binding makes a call UB -> turn that into a clean panic (covers global ctor/dtor). A `None` level with binding up
// is the legit early-Core registration window: fall back to an uncached fetch, just don't cache.
assert!(
sys::is_initialized(),
"{}::singleton() called while the Godot FFI binding is unavailable (global init/deinit). \
See is_singleton_available().",
std::any::type_name::<T>(),
);

// The pointer from global_get_singleton() is only valid while T's init level is loaded. After it unloads, Godot frees the singleton but
// keeps a dangling map entry, so dereferencing that pointer would be UB. Validate except for safeguards-disengaged. This is not a problem
// _before_ singleton is loaded; Godot correctly returns null.
if let Some(current) = current {
sys::balanced_assert!(
current >= T::INIT_LEVEL,
"{}::singleton() called after its init level was unloaded; the singleton no longer exists.\n\
Use `godot::init::is_singleton_available()` to check.",
std::any::type_name::<T>(),
);
}

let class_name = make_class_name();
// SAFETY: class_name matches T; binding initialized (asserted above).
let object_ptr = unsafe { sys::interface_fn!(global_get_singleton)(class_name.string_sys()) };

// Cache only a valid ptr with the level it was observed at; null ptr or `None` level => no valid key, don't cache (fetch still returns it).
if !object_ptr.is_null()
&& let Some(current) = current
{
// Write `ptr`/`generation` `Relaxed`, then store `level` last with `Release` (see SingletonCache docs).
cache.ptr.store(object_ptr.cast(), Ordering::Relaxed);
cache.generation.store(generation, Ordering::Relaxed);
cache.level.store(Some(current));
}

// SAFETY: null => from_obj_sys panics, identical to current behavior.
unsafe { Gd::<T>::from_obj_sys(object_ptr) }
}

/// Checks that the object with the given instance ID is still alive and that the pointer is valid.
///
/// This does **not** perform type checking — use `ensure_object_type()` for that.
Expand Down
25 changes: 24 additions & 1 deletion godot-core/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};

use godot_ffi as sys;
use sys::GodotFfi;
Expand Down Expand Up @@ -48,6 +48,23 @@ pub(crate) fn current_init_level() -> Option<InitLevel> {
CURRENT_INIT_LEVEL.load()
}

/// Counter bumped on each full deinitialization, used to invalidate caches that hold pointers from a previous init cycle.
///
/// A full deinit/reinit within the same library load would otherwise let a level-gated cache serve a stale pointer (same level, new
/// instance). Comparing the cached generation against this one structurally rejects that.
pub(crate) fn singleton_cache_generation() -> u64 {
SINGLETON_CACHE_GENERATION.load(Ordering::Acquire)
}

/// Test-only: bump the singleton-cache generation, forcing every singleton cache to miss on its next access (re-fetch via the slow path).
///
/// Mirrors what a full Core deinit does, letting itests exercise the uncached path on demand without a real deinit/reinit cycle.
#[cfg(feature = "trace")]
#[doc(hidden)]
pub fn __invalidate_singleton_caches() {
SINGLETON_CACHE_GENERATION.fetch_add(1, Ordering::Release);
}

/// Return whether a certain singleton can currently be retrieved from Godot.
///
/// This differs from [`is_class_available()`]: a singleton instance may become available later than its class API.
Expand Down Expand Up @@ -88,6 +105,9 @@ use crate::signal::prune_stored_signal_connections;

static CURRENT_INIT_LEVEL: sys::AtomicEnum<Option<InitLevel>> = sys::AtomicEnum::default();

/// See [`singleton_cache_generation()`]. Bumped once per full deinit (lowest level unloaded).
static SINGLETON_CACHE_GENERATION: AtomicU64 = AtomicU64::new(0);

#[repr(C)]
struct InitUserData {
library: sys::GDExtensionClassLibraryPtr,
Expand Down Expand Up @@ -347,6 +367,9 @@ fn gdext_on_level_deinit(level: InitLevel) {
// If lowest level is unloaded, call global deinitialization.
// No business logic by itself, but ensures consistency if re-initialization (hot-reload on Linux) occurs.

// Invalidate cached singleton pointers: a later reinit would reuse the same init level but fresh instances.
SINGLETON_CACHE_GENERATION.fetch_add(1, Ordering::Release);

crate::task::cleanup();
crate::tools::cleanup();

Expand Down
10 changes: 6 additions & 4 deletions godot-ffi/src/atomic_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use std::sync::atomic::{AtomicU8, Ordering};

/// A lock-free cell that stores a value of enum type `E` using a single byte.
///
/// All operations use [`Ordering::Relaxed`] -- sufficient for this crate (global flags not involved in acquire/release synchronisation chains).
/// Loads use [`Acquire`][Ordering::Acquire] and stores use [`Release`][Ordering::Release], so a stored value can act as a release gate that
/// publishes other (e.g. `Relaxed`) writes made before it. This is stronger than needed for plain global flags, but free on x86 and negligible
/// elsewhere, and keeps the type safe to reuse as a publishing gate.
pub struct AtomicEnum<E> {
atomic: AtomicU8,
_phantom: PhantomData<E>,
Expand All @@ -37,18 +39,18 @@ impl<E: AtomicIntLike> AtomicEnum<E> {

/// Loads the current value.
pub fn load(&self) -> E {
let raw = self.atomic.load(Ordering::Relaxed);
let raw = self.atomic.load(Ordering::Acquire);
E::from_u8(raw)
}

/// Stores a new value.
pub fn store(&self, value: E) {
self.atomic.store(value.to_u8(), Ordering::Relaxed);
self.atomic.store(value.to_u8(), Ordering::Release);
}

/// Stores a new value and returns the previous one.
pub fn replace(&self, value: E) -> E {
let old = self.atomic.swap(value.to_u8(), Ordering::Relaxed);
let old = self.atomic.swap(value.to_u8(), Ordering::AcqRel);
E::from_u8(old)
}
}
Expand Down
55 changes: 43 additions & 12 deletions itest/rust/src/object_tests/init_stage_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,25 @@ fn init_level_no_panics() {
);
}

// Asserts that T's singleton availability matches `present`. If absent, also verifies `singleton()` panics rather than handing out a
// dangling/null pointer (regression test for the deinit-dangling fix, see https://github.com/godot-rust/gdext/pull/1638).
// Asserts that T's singleton availability matches `present`. When present, also verifies that cached and uncached `singleton()` calls are
// consistent; when absent, verifies `singleton()` panics rather than handing out a dangling/null pointer (regression test, see
// https://github.com/godot-rust/gdext/pull/1638).
fn assert_singleton_present<T: Singleton + GodotClass>(present: bool) {
use godot::init::__invalidate_singleton_caches;

// First test the dedicated availability API.
assert_eq!(is_singleton_available::<T>(), present);

if present {
// Must not panic and must yield a live instance.
let _ = T::singleton().instance_id();
// Must not panic; cached and uncached fetches must resolve to the same live instance.
__invalidate_singleton_caches();
let uncached = T::singleton().instance_id();
let cached = T::singleton().instance_id();
assert_eq!(uncached, cached);

__invalidate_singleton_caches();
let uncached_again = T::singleton().instance_id();
assert_eq!(cached, uncached_again);
} else {
// Probing a missing singleton makes Godot print an error; suppress it when possible. Suppression itself goes through the Engine
// singleton, which on Godot < 4.4 is not registered before the Scene level -- skip suppression in that window.
Expand All @@ -101,6 +111,29 @@ fn assert_singleton_present<T: Singleton + GodotClass>(present: bool) {
}
}

// Verify cached singleton pointers return same live instance, never stale or wrong.
#[itest]
fn singleton_caching_stable_instance() {
let engine = Engine::singleton();
let engine_id = engine.instance_id();
assert_eq!(engine_id, Engine::singleton().instance_id());

let os = Os::singleton();
let os_id = os.instance_id();
assert_eq!(os_id, Os::singleton().instance_id());

let time_id = Time::singleton().instance_id();
assert_eq!(time_id, Time::singleton().instance_id());

assert_ne!(engine_id, os_id);
assert_ne!(engine_id, time_id);
assert_ne!(os_id, time_id);

// Functional sanity checks.
assert!(engine.get_physics_ticks_per_second() > 0);
assert!(!os.get_name().is_empty());
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Stage-specific callbacks

Expand Down Expand Up @@ -140,6 +173,7 @@ fn on_init_core() {
assert!(!is_class_available::<RenderingServer>()); // Servers

// Core singletons (Engine/Os/Time) are reachable at Core level; RenderingServer is not.
// Each call also checks cached/uncached consistency (if present) or that access panics (if absent).
assert_singleton_present::<Engine>(true);
assert_singleton_present::<Os>(true);
assert_singleton_present::<Time>(true);
Expand All @@ -158,12 +192,9 @@ fn on_init_core() {
);
}

let engine = Engine::singleton();
assert!(engine.get_physics_ticks_per_second() > 0);

let os = Os::singleton();
assert!(!os.get_name().is_empty());

// Functional: the cached pointers are usable for real method calls, not just identity-stable.
assert!(Engine::singleton().get_physics_ticks_per_second() > 0);
assert!(!Os::singleton().get_name().is_empty());
let time = Time::singleton();
assert!(time.get_ticks_usec() <= time.get_ticks_usec());
}
Expand Down Expand Up @@ -225,7 +256,7 @@ impl IObject for MainLoopCallbackSingleton {

#[cfg(since_api = "4.5")]
fn on_init_main_loop() {
// By MainLoop, the RenderingServer singleton is registered.
// By MainLoop, the RenderingServer singleton is registered. Verify availability + both cache paths.
assert!(is_class_available::<RenderingServer>());
assert_singleton_present::<RenderingServer>(true);

Expand Down Expand Up @@ -280,7 +311,7 @@ pub fn on_stage_deinit(stage: InitStage) {

#[cfg(since_api = "4.5")]
fn on_deinit_main_loop() {
// RenderingServer singleton still available at MainLoop deinit; same level still loaded on the way down.
// RenderingServer singleton still available at MainLoop deinit; same level still loaded on the way down, so both cache paths must resolve.
assert_singleton_present::<RenderingServer>(true);

let singleton = Engine::singleton()
Expand Down
Loading