Add opt-in Instance downcasting support via tracking

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" }

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<T: NativeClass> Instance<T> {
         &self.script
     }
 
+    /// Try to downcast `T::Base` to `Instance<T>`. This safe version can only be used with
+    /// reference counted base classes.
+    pub fn try_from_base(owner: T::Base) -> Option<Self>
+    where
+        T::Base: Clone,
+        T::UserData: TryClone,
+    {
+        unsafe { Self::try_from_unsafe_base(owner) }
+    }
+
+    /// Try to downcast `T::Base` to `Instance<T>`.
+    ///
+    /// # 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<Self>
+    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<F, U>(&self, op: F) -> Result<U, <T::UserData as Map>::Err>
@@ -279,6 +316,18 @@ where
     }
 }
 
+impl<T> FromVariant for Instance<T>
+where
+    T: NativeClass,
+    T::Base: FromVariant + Clone,
+    T::UserData: TryClone,
+{
+    fn from_variant(variant: &Variant) -> Option<Self> {
+        let owner = T::Base::from_variant(variant)?;
+        Self::try_from_base(owner)
+    }
+}
+
 #[macro_export]
 #[doc(hidden)]
 macro_rules! godot_class_build_export_methods {

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;
 

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<_<T>>` 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<Self>;
+}
+
+/// 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<T> = MutexData<T, DefaultLockPolicy>;
 
+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<T, OPT> UniquePtr for MutexData<T, OPT>
+where
+    T: NativeClass + Send,
+    OPT: LockOptions,
+{
+}
+
 impl<T, OPT> Clone for MutexData<T, OPT> {
     fn clone(&self) -> Self {
         MutexData {
@@ -328,6 +357,13 @@ where
     }
 }
 
+unsafe impl<T, OPT> UniquePtr for RwLockData<T, OPT>
+where
+    T: NativeClass + Send + Sync,
+    OPT: LockOptions,
+{
+}
+
 impl<T, OPT> Clone for RwLockData<T, OPT> {
     fn clone(&self) -> Self {
         RwLockData {
@@ -381,8 +417,169 @@ where
     }
 }
 
+unsafe impl<T> UniquePtr for ArcData<T> where T: NativeClass + Send + Sync {}
+
 impl<T> Clone for ArcData<T> {
     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<HashMap<TypeId, HashSet<usize>>> =
+            { 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<UD> {
+        data: UD,
+    }
+
+    unsafe impl<UD> UserData for Tracked<UD>
+    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::<UD::Target>())
+                    .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::<UD::Target>()) {
+                    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<UD> Map for Tracked<UD>
+    where
+        UD: UserData + Map + UniquePtr,
+        UD::Target: 'static,
+    {
+        type Err = UD::Err;
+
+        fn map<F, U>(&self, op: F) -> Result<U, UD::Err>
+        where
+            F: FnOnce(&UD::Target) -> U,
+        {
+            self.data.map(op)
+        }
+    }
+
+    impl<UD> MapMut for Tracked<UD>
+    where
+        UD: UserData + MapMut + UniquePtr,
+        UD::Target: 'static,
+    {
+        type Err = UD::Err;
+
+        fn map_mut<F, U>(&self, op: F) -> Result<U, UD::Err>
+        where
+            F: FnOnce(&mut UD::Target) -> U,
+        {
+            self.data.map_mut(op)
+        }
+    }
+
+    unsafe impl<UD> TryClone for Tracked<UD>
+    where
+        UD: UserData + UniquePtr,
+        UD::Target: 'static,
+    {
+        unsafe fn try_clone_from_user_data(ptr: *const libc::c_void) -> Option<Self> {
+            if ptr.is_null() {
+                return None;
+            }
+
+            let result = {
+                let ptr_map = TRACKED_POINTERS.read();
+                let ptrs = ptr_map.get(&TypeId::of::<UD::Target>())?;
+
+                // 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
+        }
+    }
+}

test/src/lib.rs

@@ -82,7 +82,7 @@ struct Foo(i64);
 
 impl NativeClass for Foo {
     type Base = Reference;
-    type UserData = user_data::ArcData<Foo>;
+    type UserData = user_data::Tracked<user_data::ArcData<Foo>>;
     fn class_name() -> &'static str {
         "Foo"
     }
@@ -92,6 +92,20 @@ impl NativeClass for Foo {
     fn register_properties(_builder: &init::ClassBuilder<Self>) {}
 }
 
+struct NotFoo;
+
+impl NativeClass for NotFoo {
+    type Base = Reference;
+    type UserData = user_data::Tracked<user_data::ArcData<NotFoo>>;
+    fn class_name() -> &'static str {
+        "NotFoo"
+    }
+    fn init(_owner: Reference) -> NotFoo {
+        NotFoo
+    }
+    fn register_properties(_builder: &init::ClassBuilder<Self>) {}
+}
+
 #[methods]
 impl Foo {
     #[export]
@@ -130,11 +144,20 @@ fn test_rust_class_construction() -> bool {
 
     let ok = std::panic::catch_unwind(|| {
         let foo = Instance::<Foo>::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::<Foo>::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::<NotFoo>::try_from_base(base).is_none());
     })
     .is_ok();