Skip to content

DisplayServer singleton error (windows) #1157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
miruji opened this issue May 6, 2025 · 6 comments
Closed

DisplayServer singleton error (windows) #1157

miruji opened this issue May 6, 2025 · 6 comments
Labels
question Not a problem with the library, but a question regarding usage.

Comments

@miruji
Copy link

miruji commented May 6, 2025

I encountered an issue with singletons - it behaves as if my class continues to hold ownership of the singleton even after it's used in a method. I even tried using references, but the behavior remains the same. The error only occurs on Windows; everything works fine on Linux. I'm using the master version of gdext. There are no compilation errors, as expected, which suggests this might be a bug specific to the Windows build. I'm targeting x86_64-pc-windows-gnu.

rust:

#[godot_api]
impl RGlobal
{
  #[func]
  fn generateResolutions(&mut self) -> ()
  {
    let displayServer: Gd<DisplayServer> = DisplayServer::singleton(); # or mut
    ...
  }
  #[func]
  fn setWindowSize(&mut self) -> ()
  {
    let mut displayServer: Gd<DisplayServer> = DisplayServer::singleton(); # ok
    displayServer.window_set_position(Vector2i::new(1366,768)); # error
    ...
  }
}

godot:

extends RGlobal
...
self.generateResolutions()
self.setWindowSize()

error:

(backtrace disabled, run application with `RUST_BACKTRACE=1` environment variable)
ERROR: [panic /home/ru/.cargo/git/checkouts/gdext-067f4b88e7bd088f/4a6f803/godot-core/sr
c/storage/mod.rs:38]
  Gd<T>::bind_mut() failed, already bound; T = game::RGlobal.
    Make sure to use `self.base_mut()` instead of `self.to_gd()` when possible.
    Details: cannot borrow while accessible mutable borrow exists.
  Context: RGlobal::setWindowSize
   at: godot_core::private::set_gdext_hook::{{closure}} (/home/ru/.cargo/git/checkouts/g
dext-067f4b88e7bd088f/4a6f803/godot-core/src/private.rs:303)
   GDScript backtrace (most recent call first):
       [0] create (res://scenes/global.gd:91)
(backtrace disabled, run application with `RUST_BACKTRACE=1` environment variable)
ERROR: [panic library\core\src\panicking.rs:218]
  panic in a function that cannot unwind
  Context: RGlobal::setWindowSize
   at: godot_core::private::set_gdext_hook::{{closure}} (/home/ru/.cargo/git/checkouts/g
dext-067f4b88e7bd088f/4a6f803/godot-core/src/private.rs:303)
   GDScript backtrace (most recent call first):
       [0] create (res://scenes/global.gd:91)
@Bromeon
Copy link
Member

Bromeon commented May 6, 2025

Are you sure this is related to the singleton? The failure in your code is bind_mut(), which applies to user objects, not singletons.

Can you please reduce the code as much as possible? I.e. see if it's possible

  • with just Object rather than DisplayServer
  • or if DisplayServer::singleton() alone is enough, no method calls on it

Please also make sure there is no ... code that may be relevant but is missing. Ideally you have a single lib.rs Rust file + single .gd file that are left unchanged, then we can also match line numbers. If you additionally provide env var RUST_BACKTRACE=1, we'll see the stacktrace.


I'm targeting x86_64-pc-windows-gnu.

We're not explicitly supporting that target, e.g. we don't run CI for it. I don't see why it should behave differently, but it's not the first time I've seen people having problems with that target on Windows.

Is there a chance you can compile to the msvc target? If it involves quite some effort, maybe wait until we sorted out the minimization of the problem first.

@miruji
Copy link
Author

miruji commented May 6, 2025

I didn’t know that x86_64-pc-windows-gnu is not supported. In any case, msvc will be better, you're right, but on Linux it’s a bit cumbersome to install everything. Therefore, I will wait with this for now.

I can't provide my full code for certain reasons, which is why I gave such a minimal example. Here’s what I managed to understand:

  1. The error appeared recently, so this logic used to work fine before, and it seems like it worked fine until now.
  2. I switched to 4.5dev3, but I checked and there is no difference with 4.5dev2, the error is the same.
  3. I checked various commits of gdext down to 0.24, and the error persists.
  4. Yes, I’m using gnu, not msvc, but it worked fine before.
  5. I checked the error with RUST_BACKTRACE=1, but I found a reference to a mut error for RGlobal, and everything fixes itself if I remove the window resizing; the error only occurs with Wine Windows.

All of this led me to the following conclusions:

  1. It looks like I changed something in the RGlobal logic, but I didn’t do that.
  2. I can get the Singleton without errors, but modifying it causes an error (changing the window size) => something is wrong with borrowing the singleton. If I remove the window resizing, the error is fixed.
  3. I found that the method generateResolutions() uses Singleton; I removed it, and everything works. => This is a borrowing issue, but I don’t store the Singleton, and logically, it should go out of ownership at the end of the method, but it seems like that doesn’t happen.

But overall, we were lucky; I was able to recreate this error exactly. It works 100% the same. Honestly, I still don't understand what's wrong. Please correct me if I made any mistake.
test.zip

@miruji
Copy link
Author

miruji commented May 6, 2025

I've concluded that this error occurs due to the use of on_notification().

@Bromeon
Copy link
Member

Bromeon commented May 6, 2025

The problem you're facing is likely related to these issues:

Problem

Thanks for the example! I don't have time to test it today or tomorrow, but it seems your problem can be reduced to this:

#[derive(GodotClass)]
#[class(init, base=Node)]
struct RGlobal {
    base: Base<Node>,
}

#[godot_api]
impl INode for RGlobal {
    fn on_notification(&mut self, what: NodeNotification) {}
}

#[godot_api]
impl RGlobal {
    #[func]
    fn setWindowSize(&mut self) {
        let mut displayServer: Gd<DisplayServer> = DisplayServer::singleton();
        displayServer.window_set_position(Vector2i::new(1366, 768));
    }
}

In other words, displayServer.window_set_position() causes a notification, which calls on_notification().
Since the calling function has an exclusive reference to self here:

fn setWindowSize(&mut self) // <-

you will get a double-borrow error when acquiring a second exclusive reference:

fn on_notification(&mut self, what: NodeNotification)
//                 ^^^^^^^^^

Solution

You can solve this by acquiring a base_mut() guard, which allows "reborrows" of &mut self. This makes re-entrant calls like the above possible.

Not tested, but can you try this?

    #[func]
    fn setWindowSize(&mut self) {
        let mut displayServer: Gd<DisplayServer> = DisplayServer::singleton();

        let _lock_self = self.base_mut(); // locks the &mut away for re-entrant exclusive access
        displayServer.window_set_position(Vector2i::new(1366, 768));
    }

@miruji
Copy link
Author

miruji commented May 6, 2025

Not tested, but can you try this?

I checked, let _lock_self = self.base_mut(); works - I think it's a good workaround.

@Bromeon Bromeon added the question Not a problem with the library, but a question regarding usage. label May 6, 2025
@Bromeon
Copy link
Member

Bromeon commented May 6, 2025

Thanks and glad it worked! I'll close this as solved then.

For possible improvements to virtual methods like on_notification, you can read #359 (comment) and subscribe to that issue 👌

Also, base_mut() is only a workaround to an extent -- in Rust it's simply not possible to simultaneously have multiple &mut references, so we need a pattern to tell the borrow checker that the reference at the caller (setWindowSize) is no longer used when it's accessed inside the callee (on_notification). While we could avoid getting a reference inside on_notification in some cases, there are still situations where it's needed, and in those, base_mut() is the idiomatic pattern to use in godot-rust. We should probably document this better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Not a problem with the library, but a question regarding usage.
Projects
None yet
Development

No branches or pull requests

2 participants