diff --git a/gdnative-core/Cargo.toml b/gdnative-core/Cargo.toml index 2876078ec..2c6b87e37 100644 --- a/gdnative-core/Cargo.toml +++ b/gdnative-core/Cargo.toml @@ -18,6 +18,7 @@ libc = "0.2" bitflags = "1.0" euclid = "0.19.6" parking_lot = "0.9.0" +lazy_static = "1.4.0" [build-dependencies] gdnative_bindings_generator = { path = "../bindings_generator", version = "0.2.0" } diff --git a/gdnative-core/src/class.rs b/gdnative-core/src/class.rs index 91af37201..9ca252d3d 100644 --- a/gdnative-core/src/class.rs +++ b/gdnative-core/src/class.rs @@ -1,6 +1,8 @@ use crate::get_api; use crate::object; use crate::sys; +use crate::user_data::TryClone; +use crate::FromVariant; use crate::GodotObject; use crate::GodotString; use crate::Instanciable; @@ -174,6 +176,41 @@ impl Instance { &self.script } + /// Try to downcast `T::Base` to `Instance`. This safe version can only be used with + /// reference counted base classes. + pub fn try_from_base(owner: T::Base) -> Option + where + T::Base: Clone, + T::UserData: TryClone, + { + unsafe { Self::try_from_unsafe_base(owner) } + } + + /// Try to downcast `T::Base` to `Instance`. + /// + /// # Safety + /// + /// It's up to the caller to ensure that `owner` points to a valid Godot object, and + /// that it will not be freed until this function returns. Otherwise, it is undefined + /// behavior to call this function and/or use its return value. + pub unsafe fn try_from_unsafe_base(owner: T::Base) -> Option + where + T::UserData: TryClone, + { + let user_data_ptr = { + let api = get_api(); + let owner_ptr = owner.to_sys(); + (api.godot_nativescript_get_userdata)(owner_ptr) as *const libc::c_void + }; + + if user_data_ptr.is_null() { + return None; + } + + let script = T::UserData::try_clone_from_user_data(user_data_ptr)?; + Some(Instance { owner, script }) + } + /// Calls a function with a NativeClass instance and its owner, and returns its return /// value. Can be used on reference counted types for multiple times. pub fn map(&self, op: F) -> Result::Err> @@ -279,6 +316,18 @@ where } } +impl FromVariant for Instance +where + T: NativeClass, + T::Base: FromVariant + Clone, + T::UserData: TryClone, +{ + fn from_variant(variant: &Variant) -> Option { + let owner = T::Base::from_variant(variant)?; + Self::try_from_base(owner) + } +} + #[macro_export] #[doc(hidden)] macro_rules! godot_class_build_export_methods { diff --git a/gdnative-core/src/lib.rs b/gdnative-core/src/lib.rs index f3cc5b18b..f69da5c03 100644 --- a/gdnative-core/src/lib.rs +++ b/gdnative-core/src/lib.rs @@ -31,6 +31,8 @@ pub extern crate libc; #[macro_use] extern crate bitflags; extern crate parking_lot; +#[macro_use] +extern crate lazy_static; pub mod geom; diff --git a/gdnative-core/src/user_data.rs b/gdnative-core/src/user_data.rs index 34e0086b1..0e749ed32 100644 --- a/gdnative-core/src/user_data.rs +++ b/gdnative-core/src/user_data.rs @@ -32,6 +32,13 @@ //! - You want fine grained lock control for parallelism. //! - All your exported methods take `&self`. //! - Your `NativeClass` type is `Send + Sync`. +//! +//! ### Use a `Tracked<_>` when: +//! +//! - You want `FromVariant` for instances of the type, so you can take them as arguments. +//! - You might have multiple GDNative libraries in one project, and want safety against foreign +//! `user_data`s that may point to arbitrary data or even invalid memory. +//! - You're fine with a global lock on instance construction, destruction, and downcasts. use parking_lot::{Mutex, RwLock}; use std::fmt::Debug; @@ -90,10 +97,25 @@ pub trait MapMut: UserData { F: FnOnce(&mut Self::Target) -> U; } +/// Trait for user-data wrappers that have a type-checked constructor. +pub unsafe trait TryClone: UserData { + /// Checked version of "cloning" constructor. This is allowed to spuriously fail, but never + /// return an invalid result. + unsafe fn try_clone_from_user_data(ptr: *const libc::c_void) -> Option; +} + +/// Marker trait for user-data wrappers that produce distinct pointers for each Godot instance. +/// +/// There is no way for the compiler to test this property, so the trait is unsafe to implement. +/// See documentation on `Tracked` for more information on why this is needed. +pub unsafe trait UniquePtr: UserData {} + /// The default user data wrapper used by derive macro, when no `user_data` attribute is present. /// This may change in the future. pub type DefaultUserData = MutexData; +pub use tracked::Tracked; + /// Error type indicating that an operation can't fail. #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] pub enum Infallible {} @@ -231,6 +253,13 @@ where } } +unsafe impl UniquePtr for MutexData +where + T: NativeClass + Send, + OPT: LockOptions, +{ +} + impl Clone for MutexData { fn clone(&self) -> Self { MutexData { @@ -328,6 +357,13 @@ where } } +unsafe impl UniquePtr for RwLockData +where + T: NativeClass + Send + Sync, + OPT: LockOptions, +{ +} + impl Clone for RwLockData { fn clone(&self) -> Self { RwLockData { @@ -381,8 +417,169 @@ where } } +unsafe impl UniquePtr for ArcData where T: NativeClass + Send + Sync {} + impl Clone for ArcData { fn clone(&self) -> Self { ArcData(self.0.clone()) } } + +mod tracked { + //! User-data wrapper adapter with type checking via tracking + + use std::any::TypeId; + use std::collections::{HashMap, HashSet}; + + use parking_lot::RwLock; + + use super::{Map, MapMut, TryClone, UniquePtr, UserData}; + + lazy_static! { + static ref TRACKED_POINTERS: RwLock>> = + { RwLock::new(HashMap::new()) }; + } + + /// A `TryClone` user-data wrapper adapter that allows instance downcasting by tracking + /// user-data pointers in a global `HashMap`. + /// + /// The `HashMap` is accessed through an `RwLock` on construction, destruction, and downcasts. + /// There is no additional runtime cost on calls from Godot. + /// + /// The cast is incomplete, as in, not all valid values will pass the type check. + /// Specifically: + /// + /// - Null pointers will always fail to type check, despite being valid pointers for ZSTs. + /// This is usually fine because user-data wrappers usually need some state, and are + /// unlikely to produce a null pointer. + /// - If the user-data is consumed, then even if the wrapped data is not dropped yet, further + /// attempts to check the same pointer will fail. This is fine because it can have no valid + /// owner in this case. + /// - If multiple instances of the underlying wrapper produce the same user-data pointer, + /// (e.g. a singleton, or a type that pulls values out of aether), then all further + /// instances will fail to check by the time the first instance is consumed. To prevent + /// this from happening, the marker trait `UniquePtr` is used as a bound on the inner + /// wrapper. Violations of `UniquePtr`'s protocol will trigger debug assertions. + #[derive(Clone, Debug)] + pub struct Tracked { + data: UD, + } + + unsafe impl UserData for Tracked + where + UD: UserData + UniquePtr, + UD::Target: 'static, + { + type Target = UD::Target; + + fn new(val: Self::Target) -> Self { + // This only creates an instance owned by Rust, so no valid objects can exist yet. + Tracked { data: UD::new(val) } + } + + unsafe fn into_user_data(self) -> *const libc::c_void { + let ptr = self.data.into_user_data() as *const libc::c_void; + { + // Only when the ownership is passed to Godot, does it become possible for an + // Godot object to be a valid instance of UD::Target. So, the pointer is added + // to the map at this point. + let mut ptr_map = TRACKED_POINTERS.write(); + let ptrs = ptr_map + .entry(TypeId::of::()) + .or_insert_with(HashSet::new); + let ptr_is_new = ptrs.insert(ptr as usize); + debug_assert!( + ptr_is_new, + "pointer obtained from into_user_data should not be in the set" + ); + } + ptr + } + + unsafe fn consume_user_data_unchecked(ptr: *const libc::c_void) -> Self { + { + // When the ownership is taken back from Godot, there can't be valid objects that + // should still be considered an instance of UD::Target. Thus, the pointer is + // removed from the map. + let mut ptr_map = TRACKED_POINTERS.write(); + match ptr_map.get_mut(&TypeId::of::()) { + Some(ptrs) => { + let ptr_is_there = ptrs.remove(&(ptr as usize)); + debug_assert!(ptr_is_there, "pointer should be in the set of UD::Target"); + } + None => { + debug_assert!(false, "pointer set should have been created by now"); + } + } + } + Tracked { + data: UD::consume_user_data_unchecked(ptr), + } + } + + unsafe fn clone_from_user_data_unchecked(ptr: *const libc::c_void) -> Self { + Tracked { + data: UD::clone_from_user_data_unchecked(ptr), + } + } + } + + impl Map for Tracked + where + UD: UserData + Map + UniquePtr, + UD::Target: 'static, + { + type Err = UD::Err; + + fn map(&self, op: F) -> Result + where + F: FnOnce(&UD::Target) -> U, + { + self.data.map(op) + } + } + + impl MapMut for Tracked + where + UD: UserData + MapMut + UniquePtr, + UD::Target: 'static, + { + type Err = UD::Err; + + fn map_mut(&self, op: F) -> Result + where + F: FnOnce(&mut UD::Target) -> U, + { + self.data.map_mut(op) + } + } + + unsafe impl TryClone for Tracked + where + UD: UserData + UniquePtr, + UD::Target: 'static, + { + unsafe fn try_clone_from_user_data(ptr: *const libc::c_void) -> Option { + if ptr.is_null() { + return None; + } + + let result = { + let ptr_map = TRACKED_POINTERS.read(); + let ptrs = ptr_map.get(&TypeId::of::())?; + + // The inner user data must be constructed before the read guard is dropped, + // because otherwise it might be consumed between the check and construction. + if ptrs.contains(&(ptr as usize)) { + Some(Tracked { + data: UD::clone_from_user_data_unchecked(ptr), + }) + } else { + None + } + }; + + result + } + } +} diff --git a/test/src/lib.rs b/test/src/lib.rs index ca78c47d4..8ad7672ea 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -82,7 +82,7 @@ struct Foo(i64); impl NativeClass for Foo { type Base = Reference; - type UserData = user_data::ArcData; + type UserData = user_data::Tracked>; fn class_name() -> &'static str { "Foo" } @@ -92,6 +92,20 @@ impl NativeClass for Foo { fn register_properties(_builder: &init::ClassBuilder) {} } +struct NotFoo; + +impl NativeClass for NotFoo { + type Base = Reference; + type UserData = user_data::Tracked>; + fn class_name() -> &'static str { + "NotFoo" + } + fn init(_owner: Reference) -> NotFoo { + NotFoo + } + fn register_properties(_builder: &init::ClassBuilder) {} +} + #[methods] impl Foo { #[export] @@ -130,11 +144,20 @@ fn test_rust_class_construction() -> bool { let ok = std::panic::catch_unwind(|| { let foo = Instance::::new(); + assert_eq!(Ok(42), foo.map(|foo, owner| { foo.answer(owner) })); + + let mut base = foo.into_base(); assert_eq!( Some(42), - unsafe { foo.into_base().call("answer".into(), &[]) }.try_to_i64() + unsafe { base.call("answer".into(), &[]) }.try_to_i64() ); + + let foo = Instance::::try_from_base(base).expect("should be able to downcast"); + assert_eq!(Ok(42), foo.map(|foo, owner| { foo.answer(owner) })); + + let base = foo.into_base(); + assert!(Instance::::try_from_base(base).is_none()); }) .is_ok();