Merge pull request #563 from jf2048/closure-watch-weak-refs

glib-macros: Pass a pointer for watched objects into closure macro

Author: sdroege

glib-macros/src/closure.rs

@@ -32,7 +32,10 @@ impl Capture {
         let alias = self.alias();
         let name = &self.name;
         match self.kind {
-            CaptureKind::Watch | CaptureKind::WeakAllowNone => quote! {
+            CaptureKind::Watch => quote! {
+                let #alias = #crate_ident::object::Watchable::watched_object(&#name);
+            },
+            CaptureKind::WeakAllowNone => quote! {
                 let #alias = #crate_ident::clone::Downgrade::downgrade(&#name);
             },
             CaptureKind::Strong => quote! {
@@ -45,7 +48,7 @@ impl Capture {
         let name = &self.name;
         match self.kind {
             CaptureKind::Watch => quote! {
-                #crate_ident::object::Object::watch_closure(&#name, &#closure_ident);
+                #crate_ident::object::Watchable::watch_closure(&#name, &#closure_ident);
             },
             _ => Default::default(),
         }
@@ -55,12 +58,9 @@ impl Capture {
         let alias = self.alias();
         match self.kind {
             CaptureKind::Watch => {
-                let err_msg = format!("failed to upgrade `{}`", alias);
                 quote! {
-                    let #alias = match #crate_ident::clone::Upgrade::upgrade(&#alias) {
-                        Some(val) => val,
-                        None => panic!(#err_msg)
-                    };
+                    let #alias = unsafe { #alias.borrow() };
+                    let #alias = ::core::convert::AsRef::as_ref(&#alias);
                 }
             }
             CaptureKind::WeakAllowNone => quote! {

glib-macros/tests/test.rs

@@ -359,7 +359,7 @@ fn closure() {
         assert_eq!(obj.ref_count(), 1);
         let weak_test = glib::closure_local!(@watch obj => move || obj.ref_count());
         assert_eq!(obj.ref_count(), 1);
-        assert_eq!(weak_test.invoke::<u32>(&[]), 3);
+        assert_eq!(weak_test.invoke::<u32>(&[]), 2);
         assert_eq!(obj.ref_count(), 1);
 
         weak_test
@@ -379,7 +379,7 @@ fn closure() {
         }
 
         let obj = glib::Object::new::<glib::Object>(&[]).unwrap();
-        assert_eq!(obj.ref_count_in_closure(), 3);
+        assert_eq!(obj.ref_count_in_closure(), 2);
     }
 
     {
@@ -398,7 +398,7 @@ fn closure() {
         let a = A {
             obj: glib::Object::new::<glib::Object>(&[]).unwrap(),
         };
-        assert_eq!(a.ref_count_in_closure(), 3);
+        assert_eq!(a.ref_count_in_closure(), 2);
     }
 
     let strong_test = {
@@ -450,4 +450,46 @@ fn closure() {
         });
         assert_eq!(struct_test.invoke::<u32>(&[]), 2);
     }
+
+    {
+        use glib::prelude::*;
+        use glib::subclass::prelude::*;
+
+        #[derive(Default)]
+        pub struct FooPrivate {}
+
+        #[glib::object_subclass]
+        impl ObjectSubclass for FooPrivate {
+            const NAME: &'static str = "MyFoo2";
+            type Type = Foo;
+        }
+
+        impl ObjectImpl for FooPrivate {}
+
+        glib::wrapper! {
+            pub struct Foo(ObjectSubclass<FooPrivate>);
+        }
+
+        impl Foo {
+            fn my_ref_count(&self) -> u32 {
+                self.ref_count()
+            }
+        }
+
+        let cast_test = {
+            let foo = glib::Object::new::<Foo>(&[]).unwrap();
+
+            assert_eq!(foo.my_ref_count(), 1);
+            let cast_test = glib::closure_local!(@watch foo => move || foo.my_ref_count());
+            assert_eq!(foo.my_ref_count(), 1);
+            assert_eq!(cast_test.invoke::<u32>(&[]), 2);
+            assert_eq!(foo.my_ref_count(), 1);
+
+            let foo_ref = &foo;
+            let _ = glib::closure_local!(@watch foo_ref => move || foo_ref.my_ref_count());
+
+            cast_test
+        };
+        cast_test.invoke::<()>(&[]);
+    }
 }

glib/src/object.rs

@@ -3168,6 +3168,53 @@ impl<T: ObjectType> ObjectExt for T {
     }
 }
 
+// Helper struct to avoid creating an extra ref on objects inside closure watches. This is safe
+// because `watch_closure` ensures the object has a ref when the closure is called.
+#[doc(hidden)]
+pub struct WatchedObject<T: ObjectType>(ptr::NonNull<T::GlibType>);
+
+#[doc(hidden)]
+unsafe impl<T: ObjectType + Send + Sync> Send for WatchedObject<T> {}
+
+#[doc(hidden)]
+impl<T: ObjectType> WatchedObject<T> {
+    pub fn new(obj: &T) -> Self {
+        Self(unsafe { ptr::NonNull::new_unchecked(obj.as_ptr()) })
+    }
+    pub unsafe fn borrow(&self) -> Borrowed<T>
+    where
+        T: FromGlibPtrBorrow<*mut <T as ObjectType>::GlibType>,
+    {
+        from_glib_borrow(self.0.as_ptr())
+    }
+}
+
+#[doc(hidden)]
+pub trait Watchable<T: ObjectType> {
+    fn watched_object(&self) -> WatchedObject<T>;
+    fn watch_closure(&self, closure: &impl AsRef<Closure>);
+}
+
+#[doc(hidden)]
+impl<T: ObjectType> Watchable<T> for T {
+    fn watched_object(&self) -> WatchedObject<T> {
+        WatchedObject::new(self)
+    }
+    fn watch_closure(&self, closure: &impl AsRef<Closure>) {
+        ObjectExt::watch_closure(self, closure)
+    }
+}
+
+#[doc(hidden)]
+impl<T: ObjectType> Watchable<T> for &T {
+    fn watched_object(&self) -> WatchedObject<T> {
+        WatchedObject::new(*self)
+    }
+    fn watch_closure(&self, closure: &impl AsRef<Closure>) {
+        ObjectExt::watch_closure(*self, closure)
+    }
+}
+
 // Validate that the given property value has an acceptable type for the given property pspec
 // and if necessary update the value
 fn validate_property_type(