Panic when using ThemeDb theme inside ScriptLanguageExtension

[aekobear]

The following extension code panics as soon as godot loads:

use godot::{
    engine::{Engine, ImageTexture, ScriptLanguageExtensionVirtual, Theme, ThemeDb},
    prelude::*,
};

struct CrashDemo;

#[gdextension]
unsafe impl ExtensionLibrary for CrashDemo {
    fn on_level_init(level: InitLevel) {
        if level != InitLevel::Editor {
            return;
        }

        // register ScriptLanguageExtension with the engine.
        let mut engine = Engine::singleton();
        let buggy_extension = Gd::<BuggyExtension>::new_default();
        let result = engine.register_script_language(buggy_extension.upcast());
        assert_eq!(result, godot::engine::global::Error::OK);
    }
}

#[derive(GodotClass)]
#[class(init, tool, base=ScriptLanguageExtension)]
struct BuggyExtension {}

#[godot_api]
impl ScriptLanguageExtensionVirtual for BuggyExtension {
    fn frame(&mut self) {
        if let Some(mut theme) = ThemeDb::singleton().get_default_theme() {
            let texture = ImageTexture::new();

            // setting icon causes a panic
            // only in &mut callbacks and only for ScriptLanguageExtensionVirtual
            theme.set_icon("Foobar".into(), "EditorIcons".into(), texture.upcast());
        }
    }

    // this needs to be defined for the crash to occur.
    // not sure if it's related to the bug, or if it's just the minimum
    // necessary to get a ScriptLanguageExtension registered with Godot.
    fn get_type(&self) -> GodotString {
        "Gibberish".into()
    }
}

the error:

thread '' panicked at /home/.../gdext-6fb575638f35006f/f813884/godot-core/src/storage.rs:86:17:
Gd::bind() failed, already bound; T = gdruby::BuggyExtension.
Make sure there is no &mut T live at the time.
This often occurs when calling a GDScript function/signal from Rust, which then calls again Rust code.
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped)

Some notes from my investigation:

• This happens in any ScriptLanguageExtension callback with a &mut self signature. It does not happen in callbacks with a &self signature, and it does not happen in callbacks from other Godot classes (I tested it in a LanguageScript callback with a &mut self signature and it worked fine)
• This only happens with the editor theme from ThemeDb. If I create a theme locally via Theme::new(), set_icon does not panic.

[lilizoey]

what happens if you run with do RUST_BACKTRACE=1?

[aekobear]

what happens if you run with do RUST_BACKTRACE=1?

Here's the full trace. I poked at the source a bit, but it seems like all the important parts are in the "" sections from 9-16 😭

thread '<unnamed>' panicked at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/storage.rs:86:17:
Gd<T>::bind() failed, already bound; T = gdruby::BuggyExtension.
  Make sure there is no &mut T live at the time.
  This often occurs when calling a GDScript function/signal from Rust, which then calls again Rust code.
stack backtrace:
   0: rust_begin_unwind
             at /rustc/1e746d7741d44551e9378daf13b8797322aa0b74/library/std/src/panicking.rs:619:5
   1: core::panicking::panic_fmt
             at /rustc/1e746d7741d44551e9378daf13b8797322aa0b74/library/core/src/panicking.rs:72:14
   2: godot_core::storage::single_threaded::InstanceStorage<T>::get::{{closure}}
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/storage.rs:86:17
   3: core::result::Result<T,E>::unwrap_or_else
             at /rustc/1e746d7741d44551e9378daf13b8797322aa0b74/library/core/src/result.rs:1429:23
   4: godot_core::storage::single_threaded::InstanceStorage<T>::get
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/storage.rs:85:13
   5: <gdruby::BuggyExtension as godot_core::obj::traits::cap::ImplementsGodotVirtual>::__virtual_call::function::{{closure}}
             at /home/aekobear/code/themedb-crash-repro/src/lib.rs:27:1
   6: core::ops::function::FnOnce::call_once
             at /rustc/1e746d7741d44551e9378daf13b8797322aa0b74/library/core/src/ops/function.rs:250:5
   7: <(R,) as godot_core::builtin::meta::signature::PtrcallSignatureTuple>::in_ptrcall
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/builtin/meta/signature.rs:278:38
   8: <gdruby::BuggyExtension as godot_core::obj::traits::cap::ImplementsGodotVirtual>::__virtual_call::function
             at /home/aekobear/code/themedb-crash-repro/src/lib.rs:27:1
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: <unknown>
  16: <unknown>
  17: <(R,P0,P1,P2) as godot_core::builtin::meta::signature::PtrcallSignatureTuple>::out_class_ptrcall::{{closure}}
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/builtin/meta/signature.rs:310:21
  18: <godot_core::builtin::meta::return_marshal::PtrcallReturnUnit as godot_core::builtin::meta::return_marshal::PtrcallReturn>::call
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/builtin/meta/return_marshal.rs:66:9
  19: <(R,P0,P1,P2) as godot_core::builtin::meta::signature::PtrcallSignatureTuple>::out_class_ptrcall
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/builtin/meta/signature.rs:309:17
  20: godot_core::gen::classes::theme::re_export::Theme::set_icon
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/gen/classes/theme.rs:78:17
  21: <gdruby::BuggyExtension as godot_core::gen::classes::script_language_extension::re_export::ScriptLanguageExtensionVirtual>::frame
             at /home/aekobear/code/themedb-crash-repro/src/lib.rs:35:13
  22: <gdruby::BuggyExtension as godot_core::obj::traits::cap::ImplementsGodotVirtual>::__virtual_call::function::{{closure}}
             at /home/aekobear/code/themedb-crash-repro/src/lib.rs:27:1
  23: core::ops::function::FnOnce::call_once
             at /rustc/1e746d7741d44551e9378daf13b8797322aa0b74/library/core/src/ops/function.rs:250:5
  24: <(R,) as godot_core::builtin::meta::signature::PtrcallSignatureTuple>::in_ptrcall
             at /home/aekobear/.local/share/rtx/installs/rust/nightly/git/checkouts/gdext-6fb575638f35006f/f813884/godot-core/src/builtin/meta/signature.rs:278:38
  25: <gdruby::BuggyExtension as godot_core::obj::traits::cap::ImplementsGodotVirtual>::__virtual_call::function
             at /home/aekobear/code/themedb-crash-repro/src/lib.rs:27:1
  26: <unknown>
  27: <unknown>
  28: __libc_start_call_main
             at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  29: __libc_start_main_impl
             at ./csu/../csu/libc-start.c:392:3
  30: <unknown>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped)

[Bromeon]

Thanks for reporting. From afar, this looks like a similar problem to #338: too eager borrowing.
Maybe set_icon() leads to a call of get_type() or so, which would violate borrow rules.

I think we can try to reproduce it in an #[itest]. The memcheck CI jobs use a specifically compiled Godot version, which should be able to display also the C++ part of the stack trace.

[Bromeon]

Should be handled by #671.