Skip to content

Commit d54125f

Browse files
committed
Handle panic in OnReady auto_init.
1 parent d2d191d commit d54125f

File tree

2 files changed

+52
-15
lines changed

2 files changed

+52
-15
lines changed

godot-core/src/obj/on_ready.rs

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -207,13 +207,9 @@ impl<T> OnReady<T> {
207207
InitState::ManualUninitialized => {
208208
self.state = InitState::Initialized { value };
209209
}
210-
InitState::AutoPrepared { .. } => {
210+
InitState::AutoPrepared { .. } | InitState::AutoInitializationFailed => {
211211
panic!("cannot call init() on auto-initialized OnReady objects")
212212
}
213-
InitState::AutoInitializing => {
214-
// SAFETY: Loading is ephemeral state that is only set in init_auto() and immediately overwritten.
215-
unsafe { std::hint::unreachable_unchecked() }
216-
}
217213
InitState::Initialized { .. } => {
218214
panic!("already initialized; did you call init() more than once?")
219215
}
@@ -223,22 +219,22 @@ impl<T> OnReady<T> {
223219
/// Runs initialization.
224220
///
225221
/// # Panics
226-
/// If the value is already initialized.
222+
/// - If the value is already initialized.
223+
/// - If previous auto initialization failed.
227224
pub(crate) fn init_auto(&mut self, base: &Gd<Node>) {
228225
// Two branches needed, because mem::replace() could accidentally overwrite an already initialized value.
229226
match &self.state {
230227
InitState::ManualUninitialized => return, // skipped
231228
InitState::AutoPrepared { .. } => {} // handled below
232-
InitState::AutoInitializing => {
233-
// SAFETY: Loading is ephemeral state that is only set below and immediately overwritten.
234-
unsafe { std::hint::unreachable_unchecked() }
229+
InitState::AutoInitializationFailed => {
230+
panic!("OnReady automatic value initialization has already failed")
235231
}
236232
InitState::Initialized { .. } => panic!("OnReady object already initialized"),
237233
};
238234

239-
// Temporarily replace with dummy state, as it's not possible to take ownership of the initializer closure otherwise.
235+
// Temporally replace with AutoInitializationFailed state which will be left in iff initialization fails.
240236
let InitState::AutoPrepared { initializer } =
241-
mem::replace(&mut self.state, InitState::AutoInitializing)
237+
mem::replace(&mut self.state, InitState::AutoInitializationFailed)
242238
else {
243239
// SAFETY: condition checked above.
244240
unsafe { std::hint::unreachable_unchecked() }
@@ -267,7 +263,9 @@ impl<T> std::ops::Deref for OnReady<T> {
267263
InitState::AutoPrepared { .. } => {
268264
panic!("OnReady automatic value uninitialized, is only available in ready()")
269265
}
270-
InitState::AutoInitializing => unreachable!(),
266+
InitState::AutoInitializationFailed => {
267+
panic!("OnReady automatic value initialization failed")
268+
}
271269
InitState::Initialized { value } => value,
272270
}
273271
}
@@ -284,7 +282,9 @@ impl<T> std::ops::DerefMut for OnReady<T> {
284282
InitState::ManualUninitialized | InitState::AutoPrepared { .. } => {
285283
panic!("value not yet initialized")
286284
}
287-
InitState::AutoInitializing => unreachable!(),
285+
InitState::AutoInitializationFailed => {
286+
panic!("OnReady automatic value initialization failed")
287+
}
288288
}
289289
}
290290
}
@@ -313,7 +313,7 @@ type InitFn<T> = dyn FnOnce(&Gd<Node>) -> T;
313313
enum InitState<T> {
314314
ManualUninitialized,
315315
AutoPrepared { initializer: Box<InitFn<T>> },
316-
AutoInitializing, // needed because state cannot be empty
316+
AutoInitializationFailed,
317317
Initialized { value: T },
318318
}
319319

@@ -324,7 +324,9 @@ impl<T: Debug> Debug for InitState<T> {
324324
InitState::AutoPrepared { .. } => {
325325
fmt.debug_struct("AutoPrepared").finish_non_exhaustive()
326326
}
327-
InitState::AutoInitializing => fmt.debug_struct("AutoInitializing").finish(),
327+
InitState::AutoInitializationFailed => {
328+
fmt.debug_struct("AutoInitializationFailed").finish()
329+
}
328330
InitState::Initialized { value } => fmt
329331
.debug_struct("Initialized")
330332
.field("value", value)

itest/rust/src/object_tests/onready_test.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
// Integration of OnReady with #[init(load = "PATH")] is tested in save_load_test.rs.
99

10+
use std::ops::{Deref, DerefMut};
11+
1012
use godot::classes::notify::NodeNotification;
1113
use godot::classes::{INode, Node};
1214
use godot::obj::{Gd, NewAlloc, OnReady};
@@ -33,6 +35,39 @@ fn onready_deref() {
3335
node.free();
3436
}
3537

38+
#[itest]
39+
fn onready_poisoned() {
40+
let node = Node::new_alloc();
41+
let mut l = OnReady::<i32>::new(|| panic!("Auto init failure."));
42+
expect_panic("Auto init must fail", || {
43+
godot::private::auto_init(&mut l, &node)
44+
});
45+
expect_panic("Calling `init` after failed auto init fails", || {
46+
l.init(44);
47+
});
48+
49+
expect_panic(
50+
"Calling `auto_init` again after failed auto init fails",
51+
|| {
52+
godot::private::auto_init(&mut l, &node);
53+
},
54+
);
55+
expect_panic(
56+
"Panic on deref after automatic initialization failure",
57+
|| {
58+
let _failed_deref = Deref::deref(&l);
59+
},
60+
);
61+
expect_panic(
62+
"Panic on deref mut after automatic initialization failure",
63+
|| {
64+
let _failed_deref = DerefMut::deref_mut(&mut l);
65+
},
66+
);
67+
68+
node.free();
69+
}
70+
3671
#[itest]
3772
fn onready_deref_on_uninit() {
3873
expect_panic("Deref on uninit fails", || {

0 commit comments

Comments
 (0)