godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
3.1k stars 198 forks source link

Nested Exported Resource requires `experimental-threads` feature #610

Open StatisMike opened 8 months ago

StatisMike commented 8 months ago

The issue was originally posted in the discord thread and some initial testing was executed. Below I will share the original question and initiali findings. Issue is similiar to #597 as it invokes the same multithreading problem with Resource handling of Godot, but contrary to the former, there is no custom ResourceFormatLoader there, only custom Resource (so it is saved as .tres and .res formats) - for this reason, I believe it is worthy to have this as a separate Issue here.

Example and initial data:

struct Test {}

#[gdextension]
unsafe impl ExtensionLibrary for Test {}

#[derive(GodotClass)]
#[class(base=Node)]
struct TestNode {
    #[export]
    nested_resources: Gd<ParentResource>,
}

#[godot_api]
impl INode for TestNode {
    fn init(_base: Base<Node>) -> Self {
        Self {
            nested_resources: Gd::from_init_fn(ParentResource::init),
        }
    }
}

#[derive(GodotClass)]
#[class(base=Resource)]
struct ParentResource {
    base: Base<Resource>,
    #[export]
    sub_resource: Gd<SubResource>,
}

#[godot_api]
impl IResource for ParentResource {
    fn init(base: Base<Resource>) -> Self {
        Self {
            base,
            sub_resource: Gd::from_init_fn(SubResource::init),
        }
    }
}

#[derive(GodotClass)]
#[class(base=Resource)]
struct SubResource {
    #[export]
    value: i32,
    base: Base<Resource>,
}

#[godot_api]
impl IResource for SubResource {
    fn init(base: Base<Resource>) -> Self {
        Self { value: 50, base }
    }
}

Above code panicked with stack trace:

<unnamed>' panicked at ~/.cargo/git/checkouts/gdext-76630c89719e160c/b4a91a6/godot-core/src/lib.rs:160:43:
no panic info available
stack backtrace:
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::panicking::panic_display
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:196:5
   3: core::panicking::panic_str
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:171:5
   4: core::option::expect_failed
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/option.rs:1980:5
   5: core::option::Option<T>::expect
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/option.rs:894:21
   6: godot_core::private::handle_panic
             at ~/.cargo/git/checkouts/gdext-76630c89719e160c/b4a91a6/godot-core/src/lib.rs:160:28
   7: <rust::ParentResource as godot_core::obj::traits::cap::ImplementsGodotExports>::__register_exports::function
             at ./rust/src/lib.rs:24:10
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: <unknown>
  16: <unknown>
  17: <unknown>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5

With error message introduced in #581

assertion `left == right` failed: attempted to access binding from different thread than main thread; this is UB - use the "experimental-threads" feature.
  left: ThreadId(1)
right: ThreadId(2)

Removing the #[export] annotation from ParentResource::sub_resource field stop the error from happening, but also removes the possibility of assigning SubResource in the Godot Editor AND saving the ParentResource with SubResource data, either as Bundled or External Resource.

Initial hypothesis and findings

My initial hypothesis was that the user code was in some way callled by Godot Editor, similiar to mentioned ResourceFormatLoader issue. To check it, the simple thread printing function was added to init function of all structs:

pub(crate) fn print_thread(class: &str, method: &str) {
    let thread_id = Os::singleton().get_thread_caller_id();

    godot_print!("Thread: {thread_id}: {class}::{method}");
}

#[godot_api]
impl IResource for SubResource {
    fn init(base: Base<Resource>) -> Self {
        print_thread("SubResource", "init");
        Self { value: 50, base }
    }
}

The thread number that were always printed was 1 though, contrary to mentioned issue, so it seems that the secondary thread was used within no user defined code (at least not explicitly).

Additionally, the same nested export situation was tested with Node-inheriting classes instead of Resource. Panic wasn't happening, so it should be specific to Resources.

StatisMike commented 8 months ago

So I've just tested it using debug build of Godot to get better backtrace. In 4.2.1:

  left: `ThreadId(1)`,
 right: `ThreadId(2)`: attempted to access binding from different thread than main thread; this is UB - use the "experimental-threads" feature.', /home/kosin/.cargo/git/checkouts/gdext-76630c89719e160c/ef563c3/godot-ffi/src/binding/single_threaded.rs:74:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/panicking.rs:67:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/panicking.rs:229:5
   4: godot_ffi::binding::single_threaded::BindingStorage::get_binding_unchecked
             at /home/kosin/.cargo/git/checkouts/gdext-76630c89719e160c/ef563c3/godot-ffi/src/binding/single_threaded.rs:74:13
   5: godot_ffi::binding::get_binding
             at /home/kosin/.cargo/git/checkouts/gdext-76630c89719e160c/ef563c3/godot-ffi/src/binding/mod.rs:105:5
   6: godot_ffi::binding::get_interface
             at /home/kosin/.cargo/git/checkouts/gdext-76630c89719e160c/ef563c3/godot-ffi/src/binding/mod.rs:191:6
   7: godot_core::private::handle_varcall_panic
             at /home/kosin/.cargo/git/checkouts/gdext-76630c89719e160c/ef563c3/godot-core/src/private.rs:237:9
   8: <rust::ParentResource as godot_core::obj::traits::cap::ImplementsGodotExports>::__register_exports::function
             at /home/kosin/Documents/GODOT/test_export_resource/rust/src/lib.rs:32:10
   9: _ZNK21GDExtensionMethodBind4callEP6ObjectPPK7VariantiRN8Callable9CallErrorE
             at /home/kosin/Documents/GODOT/godot/core/extension/gdextension.cpp:212:12
  10: _ZN7ClassDB12get_propertyEP6ObjectRK10StringNameR7Variant
             at /home/kosin/Documents/GODOT/godot/core/object/class_db.cpp:1275:34
  11: _ZNK6Object3getERK10StringNamePb
             at /home/kosin/Documents/GODOT/godot/core/object/object.cpp:345:28
  12: _ZNK8Resource19hash_edited_versionEv
             at /home/kosin/Documents/GODOT/godot/core/io/resource.cpp:353:27
  13: _ZN21EditorResourcePreview8_iterateEv
             at /home/kosin/Documents/GODOT/godot/editor/editor_resource_preview.cpp:235:63
  14: _ZN21EditorResourcePreview7_threadEv
             at /home/kosin/Documents/GODOT/godot/editor/editor_resource_preview.cpp:337:11
  15: _ZN21EditorResourcePreview12_thread_funcEPv
             at /home/kosin/Documents/GODOT/godot/editor/editor_resource_preview.cpp:102:14
  16: _ZN6Thread8callbackEmRKNS_8SettingsEPFvPvES3_
             at /home/kosin/Documents/GODOT/godot/core/os/thread.cpp:61:13
  17: _ZSt13__invoke_implIvPFvmRKN6Thread8SettingsEPFvPvES4_EJmS1_S6_S4_EET_St14__invoke_otherOT0_DpOT1_
             at /usr/include/c++/9/bits/invoke.h:60:36
  18: _ZSt8__invokeIPFvmRKN6Thread8SettingsEPFvPvES4_EJmS1_S6_S4_EENSt15__invoke_resultIT_JDpT0_EE4typeEOSA_DpOSB_
             at /usr/include/c++/9/bits/invoke.h:95:40
  19: _ZNSt6thread8_InvokerISt5tupleIJPFvmRKN6Thread8SettingsEPFvPvES6_EmS3_S8_S6_EEE9_M_invokeIJLm0ELm1ELm2ELm3ELm4EEEEvSt12_Index_tupleIJXspT_EEE
             at /usr/include/c++/9/thread:244:26
  20: _ZNSt6thread8_InvokerISt5tupleIJPFvmRKN6Thread8SettingsEPFvPvES6_EmS3_S8_S6_EEEclEv
             at /usr/include/c++/9/thread:251:31
  21: _ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvmRKN6Thread8SettingsEPFvPvES7_EmS4_S9_S7_EEEEE6_M_runEv
             at /usr/include/c++/9/thread:195:13
  22: execute_native_thread_routine
  23: start_thread
             at /build/glibc-wuryBv/glibc-2.31/nptl/pthread_create.c:477:8
  24: clone
             at /build/glibc-wuryBv/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

In 4.3 dev it didn't happen, though.

MrJoermungandr commented 2 months ago

I can confirm this still happens with nested cutom resources in godot 4.3. After compiling fine Godot just closes after selecting the resource on a node