From a1e162382efd22b57b18549473030031730ccf0b Mon Sep 17 00:00:00 2001 From: Yarvin Date: Thu, 2 Oct 2025 09:17:34 +0200 Subject: [PATCH] Handle panic in OnReady auto_init. --- godot-core/src/obj/on_ready.rs | 32 ++++++++++--------- itest/rust/src/object_tests/onready_test.rs | 35 +++++++++++++++++++++ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/godot-core/src/obj/on_ready.rs b/godot-core/src/obj/on_ready.rs index f1759ab84..ae041a3a1 100644 --- a/godot-core/src/obj/on_ready.rs +++ b/godot-core/src/obj/on_ready.rs @@ -207,13 +207,9 @@ impl OnReady { InitState::ManualUninitialized => { self.state = InitState::Initialized { value }; } - InitState::AutoPrepared { .. } => { + InitState::AutoPrepared { .. } | InitState::AutoInitializationFailed => { panic!("cannot call init() on auto-initialized OnReady objects") } - InitState::AutoInitializing => { - // SAFETY: Loading is ephemeral state that is only set in init_auto() and immediately overwritten. - unsafe { std::hint::unreachable_unchecked() } - } InitState::Initialized { .. } => { panic!("already initialized; did you call init() more than once?") } @@ -223,22 +219,22 @@ impl OnReady { /// Runs initialization. /// /// # Panics - /// If the value is already initialized. + /// - If the value is already initialized. + /// - If previous auto initialization failed. pub(crate) fn init_auto(&mut self, base: &Gd) { // Two branches needed, because mem::replace() could accidentally overwrite an already initialized value. match &self.state { InitState::ManualUninitialized => return, // skipped InitState::AutoPrepared { .. } => {} // handled below - InitState::AutoInitializing => { - // SAFETY: Loading is ephemeral state that is only set below and immediately overwritten. - unsafe { std::hint::unreachable_unchecked() } + InitState::AutoInitializationFailed => { + panic!("OnReady automatic value initialization has already failed") } InitState::Initialized { .. } => panic!("OnReady object already initialized"), }; - // Temporarily replace with dummy state, as it's not possible to take ownership of the initializer closure otherwise. + // Temporarily replace with AutoInitializationFailed state which will be left in iff initialization fails. let InitState::AutoPrepared { initializer } = - mem::replace(&mut self.state, InitState::AutoInitializing) + mem::replace(&mut self.state, InitState::AutoInitializationFailed) else { // SAFETY: condition checked above. unsafe { std::hint::unreachable_unchecked() } @@ -267,7 +263,9 @@ impl std::ops::Deref for OnReady { InitState::AutoPrepared { .. } => { panic!("OnReady automatic value uninitialized, is only available in ready()") } - InitState::AutoInitializing => unreachable!(), + InitState::AutoInitializationFailed => { + panic!("OnReady automatic value initialization failed") + } InitState::Initialized { value } => value, } } @@ -284,7 +282,9 @@ impl std::ops::DerefMut for OnReady { InitState::ManualUninitialized | InitState::AutoPrepared { .. } => { panic!("value not yet initialized") } - InitState::AutoInitializing => unreachable!(), + InitState::AutoInitializationFailed => { + panic!("OnReady automatic value initialization failed") + } } } } @@ -313,7 +313,7 @@ type InitFn = dyn FnOnce(&Gd) -> T; enum InitState { ManualUninitialized, AutoPrepared { initializer: Box> }, - AutoInitializing, // needed because state cannot be empty + AutoInitializationFailed, Initialized { value: T }, } @@ -324,7 +324,9 @@ impl Debug for InitState { InitState::AutoPrepared { .. } => { fmt.debug_struct("AutoPrepared").finish_non_exhaustive() } - InitState::AutoInitializing => fmt.debug_struct("AutoInitializing").finish(), + InitState::AutoInitializationFailed => { + fmt.debug_struct("AutoInitializationFailed").finish() + } InitState::Initialized { value } => fmt .debug_struct("Initialized") .field("value", value) diff --git a/itest/rust/src/object_tests/onready_test.rs b/itest/rust/src/object_tests/onready_test.rs index 124e91a97..918820a69 100644 --- a/itest/rust/src/object_tests/onready_test.rs +++ b/itest/rust/src/object_tests/onready_test.rs @@ -7,6 +7,8 @@ // Integration of OnReady with #[init(load = "PATH")] is tested in save_load_test.rs. +use std::ops::{Deref, DerefMut}; + use godot::classes::notify::NodeNotification; use godot::classes::{INode, Node}; use godot::obj::{Gd, NewAlloc, OnReady}; @@ -33,6 +35,39 @@ fn onready_deref() { node.free(); } +#[itest] +fn onready_poisoned() { + let node = Node::new_alloc(); + let mut l = OnReady::::new(|| panic!("Auto init failure.")); + expect_panic("Auto init must fail", || { + godot::private::auto_init(&mut l, &node) + }); + expect_panic("Calling `init` after failed auto init fails", || { + l.init(44); + }); + + expect_panic( + "Calling `auto_init` again after failed auto init fails", + || { + godot::private::auto_init(&mut l, &node); + }, + ); + expect_panic( + "Panic on deref after automatic initialization failure", + || { + let _failed_deref = Deref::deref(&l); + }, + ); + expect_panic( + "Panic on deref mut after automatic initialization failure", + || { + let _failed_deref = DerefMut::deref_mut(&mut l); + }, + ); + + node.free(); +} + #[itest] fn onready_deref_on_uninit() { expect_panic("Deref on uninit fails", || {