Skip to content

Commit 7128017

Browse files
committed
Panic handling: thread safety; set hook once and not repeatedly
2 parents e5b0772 + da23b37 commit 7128017

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+2344
-426
lines changed

.github/workflows/full-ci.yml

+17-10
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ jobs:
8585
RUSTDOCFLAGS: >
8686
-D rustdoc::broken-intra-doc-links -D rustdoc::private-intra-doc-links -D rustdoc::invalid-codeblock-attributes
8787
-D rustdoc::invalid-rust-codeblocks -D rustdoc::invalid-html-tags -D rustdoc::bare-urls -D rustdoc::unescaped-backticks
88+
-D warnings
8889
run: cargo doc -p godot --ignore-rust-version
8990

9091

@@ -255,11 +256,11 @@ jobs:
255256
godot-binary: godot.macos.editor.dev.double.x86_64
256257
rust-extra-args: --features godot/api-custom,godot/double-precision
257258

258-
- name: macos-x86-4.2
259+
- name: macos-x86-4.3
259260
os: macos-13
260-
artifact-name: macos-x86-4.2
261+
artifact-name: macos-x86-4.3
261262
godot-binary: godot.macos.editor.dev.x86_64
262-
godot-prebuilt-patch: '4.2.2'
263+
godot-prebuilt-patch: '4.3'
263264

264265
- name: macos-arm
265266
os: macos-latest
@@ -274,11 +275,11 @@ jobs:
274275
# godot-binary: godot.macos.editor.dev.double.arm64
275276
# rust-extra-args: --features godot/api-custom,godot/double-precision
276277

277-
- name: macos-arm-4.2
278+
- name: macos-arm-4.3
278279
os: macos-latest
279-
artifact-name: macos-arm-4.2
280+
artifact-name: macos-arm-4.3
280281
godot-binary: godot.macos.editor.dev.arm64
281-
godot-prebuilt-patch: '4.2.2'
282+
godot-prebuilt-patch: '4.3'
282283

283284
# Windows
284285

@@ -294,11 +295,11 @@ jobs:
294295
godot-binary: godot.windows.editor.dev.double.x86_64.exe
295296
rust-extra-args: --features godot/api-custom,godot/double-precision
296297

297-
- name: windows-4.2
298+
- name: windows-4.3
298299
os: windows-latest
299-
artifact-name: windows-4.2
300+
artifact-name: windows-4.3
300301
godot-binary: godot.windows.editor.dev.x86_64.exe
301-
godot-prebuilt-patch: '4.2.2'
302+
godot-prebuilt-patch: '4.3'
302303

303304
# - name: windows-4.1
304305
# os: windows-latest
@@ -342,6 +343,12 @@ jobs:
342343

343344
# Linux compat (4.1 disabled, already covered by memcheck)
344345

346+
- name: linux-4.4
347+
os: ubuntu-22.04
348+
artifact-name: linux-4.4
349+
godot-binary: godot.linuxbsd.editor.dev.x86_64
350+
godot-prebuilt-patch: '4.4'
351+
345352
- name: linux-4.3
346353
os: ubuntu-22.04
347354
artifact-name: linux-4.3
@@ -422,7 +429,7 @@ jobs:
422429
- name: "Install Godot"
423430
uses: ./.github/composite/godot-install
424431
with:
425-
artifact-name: godot-linux-4.3
432+
artifact-name: godot-linux-4.4
426433
godot-binary: godot.linuxbsd.editor.dev.x86_64
427434

428435
- name: "Run examples for short time"

.github/workflows/minimal-ci.yml

+13-7
Original file line numberDiff line numberDiff line change
@@ -171,23 +171,29 @@ jobs:
171171

172172
# Linux compat
173173

174-
- name: linux-4.1
174+
- name: linux-4.4
175175
os: ubuntu-22.04
176-
artifact-name: linux-4.1
176+
artifact-name: linux-4.4
177177
godot-binary: godot.linuxbsd.editor.dev.x86_64
178-
godot-prebuilt-patch: '4.1.4'
178+
godot-prebuilt-patch: '4.4'
179+
180+
- name: linux-4.3
181+
os: ubuntu-22.04
182+
artifact-name: linux-4.3
183+
godot-binary: godot.linuxbsd.editor.dev.x86_64
184+
godot-prebuilt-patch: '4.3'
179185

180186
- name: linux-4.2
181187
os: ubuntu-22.04
182188
artifact-name: linux-4.2
183189
godot-binary: godot.linuxbsd.editor.dev.x86_64
184190
godot-prebuilt-patch: '4.2.2'
185191

186-
- name: linux-4.3
192+
- name: linux-4.1
187193
os: ubuntu-22.04
188-
artifact-name: linux-4.3
194+
artifact-name: linux-4.1
189195
godot-binary: godot.linuxbsd.editor.dev.x86_64
190-
godot-prebuilt-patch: '4.3'
196+
godot-prebuilt-patch: '4.1.4'
191197

192198
# Memory checkers
193199

@@ -239,7 +245,7 @@ jobs:
239245
- name: "Install Godot"
240246
uses: ./.github/composite/godot-install
241247
with:
242-
artifact-name: godot-linux-4.3
248+
artifact-name: godot-linux-4.4
243249
godot-binary: godot.linuxbsd.editor.dev.x86_64
244250

245251
- name: "Run examples for short time"

check.sh

+4-1
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ function cmd_clippy() {
165165
}
166166

167167
function cmd_klippy() {
168+
# TODO(Rust 1.86): remove `-A clippy::precedence`.
169+
168170
run cargo clippy --fix --all-targets "${extraCargoArgs[@]}" -- \
169171
-D clippy::suspicious \
170172
-D clippy::style \
@@ -173,7 +175,8 @@ function cmd_klippy() {
173175
-D clippy::dbg_macro \
174176
-D clippy::todo \
175177
-D clippy::unimplemented \
176-
-D warnings
178+
-D warnings \
179+
-A clippy::precedence
177180
}
178181

179182
function cmd_test() {

godot-bindings/Cargo.toml

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ api-4-2 = []
2828
api-4-2-1 = []
2929
api-4-2-2 = []
3030
api-4-3 = []
31+
api-4-4 = []
3132
# ]]
3233

3334
default = []
@@ -36,7 +37,7 @@ api-custom = ["dep:bindgen", "dep:regex", "dep:which"]
3637
api-custom-extheader = []
3738

3839
[dependencies]
39-
gdextension-api = { version = "0.2.1", git = "https://github.com/godot-rust/godot4-prebuilt", branch = "releases" }
40+
gdextension-api = { version = "0.2.2", git = "https://github.com/godot-rust/godot4-prebuilt", branch = "releases" }
4041

4142
# Do not use bindgen 0.69, it contains regression that forces recompilation of code.
4243
bindgen = { optional = true, version = "0.68", default-features = false, features = ["runtime"] }

godot-bindings/build.rs

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ fn main() {
2525
if cfg!(feature = "api-4-2-1") { count += 1; }
2626
if cfg!(feature = "api-4-2-2") { count += 1; }
2727
if cfg!(feature = "api-4-3") { count += 1; }
28+
if cfg!(feature = "api-4-4") { count += 1; }
2829
// ]]
2930

3031
assert!(count <= 1, "ERROR: at most one `api-*` feature can be enabled");

godot-bindings/src/import.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub const ALL_VERSIONS: &[(u8, u8, u8)] = &[
2525
(4, 2, 2),
2626
(4, 3, 0),
2727
(4, 4, 0),
28+
(4, 5, 0),
2829
// ]]
2930
];
3031

@@ -57,6 +58,9 @@ pub use gdextension_api::version_4_2_2 as prebuilt;
5758
#[cfg(feature = "api-4-3")]
5859
pub use gdextension_api::version_4_3 as prebuilt;
5960

61+
#[cfg(feature = "api-4-4")]
62+
pub use gdextension_api::version_4_4 as prebuilt;
63+
6064
// ]]
6165

6266
// If none of the api-* features are provided, use default prebuilt version (typically latest Godot stable release).
@@ -75,12 +79,13 @@ pub use gdextension_api::version_4_3 as prebuilt;
7579
feature = "api-4-2-1",
7680
feature = "api-4-2-2",
7781
feature = "api-4-3",
82+
feature = "api-4-4",
7883
feature = "api-custom",
7984
)))]
8085
// ]]
8186

8287
// [version-sync] [[
8388
// [include] current.minor
8489
// [line] pub use gdextension_api::version_$snakeVersion as prebuilt;
85-
pub use gdextension_api::version_4_3 as prebuilt;
90+
pub use gdextension_api::version_4_4 as prebuilt;
8691
// ]]

godot-core/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ api-4-2 = ["godot-ffi/api-4-2"]
3838
api-4-2-1 = ["godot-ffi/api-4-2-1"]
3939
api-4-2-2 = ["godot-ffi/api-4-2-2"]
4040
api-4-3 = ["godot-ffi/api-4-3"]
41+
api-4-4 = ["godot-ffi/api-4-4"]
4142
# ]]
4243

4344
[dependencies]

godot-core/src/builtin/callable.rs

+85-37
Original file line numberDiff line numberDiff line change
@@ -64,58 +64,55 @@ impl Callable {
6464
}
6565
}
6666

67+
/// Create a callable for a method on any [`Variant`].
68+
///
69+
/// Allows to dynamically call methods on builtin types (e.g. `String.md5_text`). Note that Godot method names are used, not Rust ones.
70+
/// If the variant type is `Object`, the behavior will match that of `from_object_method()`.
71+
///
72+
/// If the builtin type does not have the method, the returned callable will be invalid.
73+
///
74+
/// Static builtin methods (e.g. `String.humanize_size`) are not supported in reflection as of Godot 4.4. For static _class_ functions,
75+
/// use [`from_local_static()`][Self::from_local_static] instead.
76+
///
77+
/// _Godot equivalent: `Callable.create(Variant variant, StringName method)`_
78+
#[cfg(since_api = "4.3")]
79+
pub fn from_variant_method<S>(variant: &Variant, method_name: S) -> Self
80+
where
81+
S: meta::AsArg<StringName>,
82+
{
83+
meta::arg_into_ref!(method_name);
84+
inner::InnerCallable::create(variant, method_name)
85+
}
86+
6787
/// Create a callable for the static method `class_name::function` (single-threaded).
6888
///
6989
/// Allows you to call static functions through `Callable`.
7090
///
71-
/// Note that due to varying support across different engine versions, the resulting `Callable` has unspecified behavior for
72-
/// methods such as [`method_name()`][Self::method_name], [`object()`][Self::object], [`object_id()`][Self::object_id] or
73-
/// [`get_argument_count()`][Self::arg_len] among others. It is recommended to only use this for calling the function.
91+
/// Does not support built-in types (such as `String`), only classes.
7492
///
7593
/// # Compatibility
76-
/// Up until and including Godot 4.3, this method has some limitations:
77-
/// - [`is_valid()`][Self::is_valid] will return `false`, even though the call itself succeeds.
78-
/// - You cannot use statics to connect signals to such callables. Use the new typed signal API instead.
94+
/// Not available before Godot 4.4. Library versions <0.3 used to provide this, however the polyfill used to emulate it was half-broken
95+
/// (not supporting signals, bind(), method_name(), is_valid(), etc).
96+
#[cfg(since_api = "4.4")]
7997
pub fn from_local_static(
8098
class_name: impl meta::AsArg<StringName>,
8199
function_name: impl meta::AsArg<StringName>,
82100
) -> Self {
83101
meta::arg_into_owned!(class_name);
84102
meta::arg_into_owned!(function_name);
85103

86-
// Modern implementation: use ClassDb::class_call_static().
87-
#[cfg(since_api = "4.4")]
88-
{
89-
let callable_name = format!("{class_name}.{function_name}");
90-
91-
Self::from_local_fn(&callable_name, move |args| {
92-
let args = args.iter().cloned().cloned().collect::<Vec<_>>();
104+
let callable_name = format!("{class_name}.{function_name}");
93105

94-
let result: Variant = classes::ClassDb::singleton().class_call_static(
95-
&class_name,
96-
&function_name,
97-
args.as_slice(),
98-
);
99-
Ok(result)
100-
})
101-
}
106+
Self::from_local_fn(&callable_name, move |args| {
107+
let args = args.iter().cloned().cloned().collect::<Vec<_>>();
102108

103-
// Polyfill for <= Godot 4.3: use GDScript expressions.
104-
#[cfg(before_api = "4.4")]
105-
{
106-
use crate::obj::NewGd;
107-
108-
let code = format!(
109-
"static func __callable():\n\treturn Callable({class_name}, \"{function_name}\")"
109+
let result: Variant = classes::ClassDb::singleton().class_call_static(
110+
&class_name,
111+
&function_name,
112+
args.as_slice(),
110113
);
111-
112-
let mut script = classes::GDScript::new_gd();
113-
script.set_source_code(&code);
114-
script.reload();
115-
116-
let callable = script.call("__callable", &[]);
117-
callable.to()
118-
}
114+
Ok(result)
115+
})
119116
}
120117

121118
#[cfg(since_api = "4.2")]
@@ -125,7 +122,7 @@ impl Callable {
125122
token: ptr::null_mut(),
126123
object_id: 0,
127124
call_func: None,
128-
is_valid_func: None, // could be customized, but no real use case yet.
125+
is_valid_func: None, // overwritten later.
129126
free_func: None,
130127
hash_func: None,
131128
equal_func: None,
@@ -158,6 +155,24 @@ impl Callable {
158155
})
159156
}
160157

158+
#[cfg(since_api = "4.2")]
159+
pub(crate) fn with_scoped_fn<S, F, Fc, R>(name: S, rust_function: F, callable_usage: Fc) -> R
160+
where
161+
S: meta::AsArg<GString>,
162+
F: FnMut(&[&Variant]) -> Result<Variant, ()>,
163+
Fc: FnOnce(&Callable) -> R,
164+
{
165+
meta::arg_into_owned!(name);
166+
167+
let callable = Self::from_fn_wrapper(FnWrapper {
168+
rust_function,
169+
name,
170+
thread_id: Some(std::thread::current().id()),
171+
});
172+
173+
callable_usage(&callable)
174+
}
175+
161176
/// Create callable from **thread-safe** Rust function or closure.
162177
///
163178
/// `name` is used for the string representation of the closure, which helps debugging.
@@ -219,12 +234,14 @@ impl Callable {
219234
let userdata = CallableUserdata { inner: callable };
220235

221236
let info = CallableCustomInfo {
237+
// We could technically associate an object_id with the custom callable. is_valid_func would then check that for validity.
222238
callable_userdata: Box::into_raw(Box::new(userdata)) as *mut std::ffi::c_void,
223239
call_func: Some(rust_callable_call_custom::<C>),
224240
free_func: Some(rust_callable_destroy::<C>),
225241
hash_func: Some(rust_callable_hash::<C>),
226242
equal_func: Some(rust_callable_equal::<C>),
227243
to_string_func: Some(rust_callable_to_string_display::<C>),
244+
is_valid_func: Some(rust_callable_is_valid_custom::<C>),
228245
..Self::default_callable_custom_info()
229246
};
230247

@@ -243,6 +260,7 @@ impl Callable {
243260
call_func: Some(rust_callable_call_fn::<F>),
244261
free_func: Some(rust_callable_destroy::<FnWrapper<F>>),
245262
to_string_func: Some(rust_callable_to_string_named::<F>),
263+
is_valid_func: Some(rust_callable_is_valid),
246264
..Self::default_callable_custom_info()
247265
};
248266

@@ -530,6 +548,17 @@ mod custom_callable {
530548
/// Error handling is mostly needed in case argument number or types mismatch.
531549
#[allow(clippy::result_unit_err)] // TODO remove once there's a clear error type here.
532550
fn invoke(&mut self, args: &[&Variant]) -> Result<Variant, ()>;
551+
552+
// TODO(v0.3): add object_id().
553+
554+
/// Returns whether the callable is considered valid.
555+
///
556+
/// True by default.
557+
///
558+
/// If this Callable stores an object, this method should return whether that object is alive.
559+
fn is_valid(&self) -> bool {
560+
true
561+
}
533562
}
534563

535564
pub unsafe extern "C" fn rust_callable_call_custom<C: RustCallable>(
@@ -641,4 +670,23 @@ mod custom_callable {
641670
w.name.clone().move_into_string_ptr(r_out);
642671
*r_is_valid = sys::conv::SYS_TRUE;
643672
}
673+
674+
// Implementing this is necessary because the default (nullptr) may consider custom callables as invalid in some cases.
675+
pub unsafe extern "C" fn rust_callable_is_valid_custom<C: RustCallable>(
676+
callable_userdata: *mut std::ffi::c_void,
677+
) -> sys::GDExtensionBool {
678+
let w: &mut C = CallableUserdata::inner_from_raw(callable_userdata);
679+
let valid = w.is_valid();
680+
681+
sys::conv::bool_to_sys(valid)
682+
}
683+
684+
// Implementing this is necessary because the default (nullptr) may consider custom callables as invalid in some cases.
685+
pub unsafe extern "C" fn rust_callable_is_valid(
686+
_callable_userdata: *mut std::ffi::c_void,
687+
) -> sys::GDExtensionBool {
688+
// If we had an object (CallableCustomInfo::object_id field), we could check whether that object is alive.
689+
// But since we just take a Rust function/closure, not knowing what happens inside, we assume always valid.
690+
sys::conv::SYS_TRUE
691+
}
644692
}

0 commit comments

Comments
 (0)