Skip to content

Commit

Permalink
Merge pull request #1045 from TitanNano/jovan/main_thread_id
Browse files Browse the repository at this point in the history
Global main thread ID
  • Loading branch information
Bromeon authored Feb 12, 2025
2 parents 187a289 + 9718ffc commit b2bff88
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 32 deletions.
1 change: 1 addition & 0 deletions godot-core/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ fn main() {
println!("cargo:rerun-if-changed=build.rs");

godot_bindings::emit_godot_version_cfg();
godot_bindings::emit_wasm_nothreads_cfg();
}
3 changes: 3 additions & 0 deletions godot-core/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ use crate::out;

pub use sys::GdextBuild;

#[cfg(not(wasm_nothreads))]
pub use sys::{is_main_thread, main_thread_id};

#[doc(hidden)]
#[deny(unsafe_op_in_unsafe_fn)]
pub unsafe fn __gdext_load_library<E: ExtensionLibrary>(
Expand Down
34 changes: 2 additions & 32 deletions godot-ffi/src/binding/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,13 @@
use std::cell::Cell;

#[cfg(not(wasm_nothreads))]
use std::thread::ThreadId;

use super::GodotBinding;
use crate::ManualInitCell;

pub(super) struct BindingStorage {
// No threading when linking against Godot with a nothreads Wasm build.
// Therefore, we just need to check if the bindings were initialized, as all accesses are from the main thread.
#[cfg(wasm_nothreads)]
initialized: Cell<bool>,

// Is used in to check that we've been called from the right thread, so must be thread-safe to access.
#[cfg(not(wasm_nothreads))]
main_thread_id: Cell<Option<ThreadId>>,
binding: ManualInitCell<GodotBinding>,
}

Expand All @@ -38,11 +30,7 @@ impl BindingStorage {
#[inline(always)]
unsafe fn storage() -> &'static Self {
static BINDING: BindingStorage = BindingStorage {
#[cfg(wasm_nothreads)]
initialized: Cell::new(false),

#[cfg(not(wasm_nothreads))]
main_thread_id: Cell::new(None),
binding: ManualInitCell::new(),
};

Expand All @@ -53,11 +41,7 @@ impl BindingStorage {
///
/// It is recommended to use this function for that purpose as the field to check varies depending on the compilation target.
fn initialized(&self) -> bool {
#[cfg(wasm_nothreads)]
return self.initialized.get();

#[cfg(not(wasm_nothreads))]
self.main_thread_id.get().is_some()
self.initialized.get()
}

/// Marks the binding storage as initialized or deinitialized.
Expand All @@ -78,17 +62,7 @@ impl BindingStorage {
}
}

// 'std::thread::current()' fails when linking to a Godot web build without threads. When compiling to wasm-nothreads,
// we assume it is impossible to have multi-threading, so checking if we are in the main thread is not needed.
// Therefore, we don't store the thread ID, but rather just whether initialization already occurred.
#[cfg(wasm_nothreads)]
self.initialized.set(initialized);

#[cfg(not(wasm_nothreads))]
{
let thread_id = initialized.then(|| std::thread::current().id());
self.main_thread_id.set(thread_id);
}
}

/// Initialize the binding storage, this must be called before any other public functions.
Expand Down Expand Up @@ -152,11 +126,7 @@ impl BindingStorage {
// TODO: figure out why the panic happens on Android, and how to resolve it. See https://github.com/godot-rust/gdext/pull/780.
#[cfg(all(debug_assertions, not(wasm_nothreads), not(target_os = "android")))]
{
let main_thread_id = storage.main_thread_id.get().expect(
"Godot engine not available; make sure you are not calling it from unit/doc tests",
);

if main_thread_id != std::thread::current().id() {
if !crate::is_main_thread() {
// If a binding is accessed the first time, this will panic and start unwinding. It can then happen that during unwinding,
// another FFI call happens (e.g. Godot destructor), which would cause immediate abort, swallowing the error message.
// Thus check if a panic is already in progress.
Expand Down
37 changes: 37 additions & 0 deletions godot-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ use binding::{
initialize_class_scene_method_table, initialize_class_server_method_table, runtime_metadata,
};

#[cfg(not(wasm_nothreads))]
static MAIN_THREAD_ID: ManualInitCell<std::thread::ThreadId> = ManualInitCell::new();

/// Stage of the Godot initialization process.
///
/// Godot's initialization and deinitialization processes are split into multiple stages, like a stack. At each level,
Expand Down Expand Up @@ -167,6 +170,14 @@ pub unsafe fn initialize(
GdextBuild::godot_static_version_string()
);

// We want to initialize the main thread ID as early as possible.
//
// SAFETY: We set the main thread ID exactly once here and never again.
#[cfg(not(wasm_nothreads))]
unsafe {
MAIN_THREAD_ID.set(std::thread::current().id())
};

// Before anything else: if we run into a Godot binary that's compiled differently from gdext, proceeding would be UB -> panic.
interface_init::ensure_static_runtime_compatibility(get_proc_address);

Expand Down Expand Up @@ -380,6 +391,32 @@ pub unsafe fn godot_has_feature(
return_ptr
}

/// Get the [`ThreadId`](std::thread::ThreadId) of the main thread.
///
/// # Panics
/// - If it is called before the engine bindings have been initialized.
#[cfg(not(wasm_nothreads))]
pub fn main_thread_id() -> std::thread::ThreadId {
assert!(
MAIN_THREAD_ID.is_initialized(),
"Godot engine not available; make sure you are not calling it from unit/doc tests"
);

// SAFETY: We initialized the cell during library initialization, before any other code is executed.
let thread_id = unsafe { MAIN_THREAD_ID.get_unchecked() };

*thread_id
}

/// Check if the current thread is the main thread.
///
/// # Panics
/// - If it is called before the engine bindings have been initialized.
#[cfg(not(wasm_nothreads))]
pub fn is_main_thread() -> bool {
std::thread::current().id() == main_thread_id()
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Macros to access low-level function bindings

Expand Down

0 comments on commit b2bff88

Please sign in to comment.