Skip to content

Commit 329122d

Browse files
committed
Remove handling of "surplus strong ref" in InstanceStorage
Currently no purpose.
1 parent 221c755 commit 329122d

File tree

5 files changed

+6
-38
lines changed

5 files changed

+6
-38
lines changed

godot-core/src/obj/base.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ impl<T: GodotClass> Base<T> {
204204
}
205205

206206
/// Finalizes the initialization of this `Base<T>` and returns whether
207-
pub(crate) fn mark_initialized(&mut self) -> bool {
207+
pub(crate) fn mark_initialized(&mut self) {
208208
#[cfg(debug_assertions)]
209209
{
210210
assert_eq!(
@@ -216,7 +216,7 @@ impl<T: GodotClass> Base<T> {
216216
self.init_state.set(InitState::ObjectInitialized);
217217
}
218218

219-
self.extra_strong_ref.borrow().is_some()
219+
// May return whether there is a "surplus" strong ref in the future, as self.extra_strong_ref.borrow().is_some().
220220
}
221221

222222
/// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed.

godot-core/src/registry/callbacks.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,7 @@ where
154154
}
155155

156156
// Mark initialization as complete, now that user constructor has finished.
157-
if base_copy.mark_initialized() {
158-
// If an extra RefCounted reference was handed out, notify storage about it, so it can unreference later.
159-
let instance_ref = unsafe { &*instance_rust_ptr };
160-
instance_ref.mark_surplus_ref();
161-
}
157+
base_copy.mark_initialized();
162158

163159
// No std::mem::forget(base_copy) here, since Base may stores other fields that need deallocation.
164160
Ok(instance_ptr)

godot-core/src/storage/instance_storage.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,6 @@ pub unsafe trait Storage {
139139
log_pre_drop(self);
140140
}
141141

142-
/// For ref-counted objects, marks as owning a surplus reference.
143-
///
144-
/// Needed when a `Base<T>` hands out extra `Gd<T>` pointers during `init()`, which then requires upgrading the `Base`
145-
/// weak pointer to a strong one. To compensate, this bool flag will skip the first `inc_ref()` call, which is typically
146-
/// object construction.
147-
fn mark_surplus_ref(&self);
148-
149142
/*#[inline(always)]
150143
fn destroyed_by_godot(&self) -> bool {
151144
out!(

godot-core/src/storage/multi_threaded.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
77

8-
use std::sync::atomic::{AtomicBool, Ordering};
9-
108
#[cfg(feature = "experimental-threads")]
119
use godot_cell::blocking::{GdCell, InaccessibleGuard, MutGuard, RefGuard};
1210
#[cfg(not(feature = "experimental-threads"))]
@@ -21,7 +19,6 @@ pub struct InstanceStorage<T: GodotClass> {
2119

2220
// Declared after `user_instance`, is dropped last
2321
pub(super) lifecycle: AtomicLifecycle,
24-
has_surplus_ref: AtomicBool,
2522

2623
// No-op in Release mode.
2724
borrow_tracker: DebugBorrowTracker,
@@ -48,7 +45,6 @@ unsafe impl<T: GodotClass> Storage for InstanceStorage<T> {
4845
user_instance: GdCell::new(user_instance),
4946
base,
5047
lifecycle: AtomicLifecycle::new(Lifecycle::Alive),
51-
has_surplus_ref: AtomicBool::new(false),
5248
borrow_tracker: DebugBorrowTracker::new(),
5349
}
5450
}
@@ -57,10 +53,6 @@ unsafe impl<T: GodotClass> Storage for InstanceStorage<T> {
5753
self.user_instance.is_currently_bound()
5854
}
5955

60-
fn mark_surplus_ref(&self) {
61-
self.has_surplus_ref.store(true, Ordering::Relaxed);
62-
}
63-
6456
fn base(&self) -> &Base<<Self::Instance as GodotClass>::Base> {
6557
&self.base
6658
}

godot-core/src/storage/single_threaded.rs

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ pub struct InstanceStorage<T: GodotClass> {
2121

2222
// Declared after `user_instance`, is dropped last.
2323
pub(super) lifecycle: cell::Cell<Lifecycle>,
24-
has_surplus_ref: cell::Cell<bool>,
2524

2625
// No-op in Release mode.
2726
borrow_tracker: DebugBorrowTracker,
@@ -48,7 +47,6 @@ unsafe impl<T: GodotClass> Storage for InstanceStorage<T> {
4847
user_instance: GdCell::new(user_instance),
4948
base,
5049
lifecycle: cell::Cell::new(Lifecycle::Alive),
51-
has_surplus_ref: cell::Cell::new(false),
5250
borrow_tracker: DebugBorrowTracker::new(),
5351
}
5452
}
@@ -57,10 +55,6 @@ unsafe impl<T: GodotClass> Storage for InstanceStorage<T> {
5755
self.user_instance.is_currently_bound()
5856
}
5957

60-
fn mark_surplus_ref(&self) {
61-
self.has_surplus_ref.set(true);
62-
}
63-
6458
fn base(&self) -> &Base<<Self::Instance as GodotClass>::Base> {
6559
&self.base
6660
}
@@ -105,25 +99,18 @@ unsafe impl<T: GodotClass> Storage for InstanceStorage<T> {
10599

106100
impl<T: GodotClass> StorageRefCounted for InstanceStorage<T> {
107101
fn on_inc_ref(&self) {
108-
if self.has_surplus_ref.get() {
109-
self.has_surplus_ref.set(false);
110-
111-
return; // If we have surplus references, we don't increment the ref count.
112-
}
102+
// Note: on_inc_ref() and on_dec_ref() do not track extra strong references from Base::to_init_gd().
103+
// See https://github.com/godot-rust/gdext/pull/1273 for code that had it.
113104

114105
super::log_inc_ref(self);
115106
}
116107

117108
fn on_dec_ref(&self) {
118-
// IMPORTANT: it is too late here to perform dec-ref operations on the Base (for "surplus" references).
109+
// IMPORTANT: it is too late here to perform dec-ref operations on the Base (for "surplus" strong references).
119110
// This callback is only invoked in the C++ condition `if (rc_val <= 1 /* higher is not relevant */)` -- see Godot ref_counted.cpp.
120111
// The T <-> RefCounted hierarchical relation is usually already broken up at this point, and further dec-ref may bring the count
121112
// down to 0.
122113

123-
if self.has_surplus_ref.get() {
124-
self.has_surplus_ref.set(false);
125-
}
126-
127114
super::log_dec_ref(self);
128115
}
129116
}

0 commit comments

Comments
 (0)