From 05c4b24ae5d8bd95d5de5d9d18fd69a1b364e3c7 Mon Sep 17 00:00:00 2001 From: fbrouille Date: Wed, 30 Aug 2023 18:59:29 +0200 Subject: [PATCH] review how to handle TypeModule lifecycle Signed-off-by: fbrouille --- glib-macros/tests/test.rs | 14 ++++++------- glib/src/lib.rs | 1 - glib/src/prelude.rs | 1 - glib/src/subclass/type_module.rs | 13 +++++++++++- glib/src/type_module.rs | 34 -------------------------------- 5 files changed, 19 insertions(+), 44 deletions(-) delete mode 100644 glib/src/type_module.rs diff --git a/glib-macros/tests/test.rs b/glib-macros/tests/test.rs index 73c19abb0346..7d5165de09ac 100644 --- a/glib-macros/tests/test.rs +++ b/glib-macros/tests/test.rs @@ -492,9 +492,9 @@ fn module_subclassable() { assert!(!module::imp::MyModuleTypeLazy::type_().is_valid()); // simulate the glib type system to load/unload the module - let module = module::MyModule::new(); - TypeModuleExt::use_(module.as_ref()); - TypeModuleExt::unuse(module.as_ref()); + let module = glib::Object::new::(); + TypeModuleExt::use_(&module); + TypeModuleExt::unuse(&module); // check types of module types and of module interfaces that are immediately registered are valid (module was loaded) assert!(module::imp::MyModuleInterface::type_().is_valid()); @@ -504,7 +504,7 @@ fn module_subclassable() { assert!(!module::imp::MyModuleTypeLazy::type_().is_valid()); // simulate the glib type system to load the module - TypeModuleExt::use_(module.as_ref()); + TypeModuleExt::use_(&module); // check types of module types and of module interfaces are valid (module is loaded) let iface_type = module::imp::MyModuleInterface::type_(); @@ -535,7 +535,7 @@ fn module_subclassable() { ); // simulate the glib type system to unload the module - TypeModuleExt::unuse(module.as_ref()); + TypeModuleExt::unuse(&module); // check types of module types and of module interfaces are still valid (should have been marked as unloaded by the glib type system but this cannot be checked) assert!(module::imp::MyModuleInterface::type_().is_valid()); @@ -544,7 +544,7 @@ fn module_subclassable() { assert!(module::imp::MyModuleTypeLazy::type_().is_valid()); // simulate the glib type system to reload the module - TypeModuleExt::use_(module.as_ref()); + TypeModuleExt::use_(&module); // check types of module types and of module interfaces are still valid (should have been marked as loaded by the glib type system but this cannot be checked) assert!(module::imp::MyModuleInterface::type_().is_valid()); @@ -553,7 +553,7 @@ fn module_subclassable() { assert!(module::imp::MyModuleTypeLazy::type_().is_valid()); // simulate the glib type system to unload the module - TypeModuleExt::unuse(module.as_ref()); + TypeModuleExt::unuse(&module); } #[test] diff --git a/glib/src/lib.rs b/glib/src/lib.rs index 2e8bde824f10..5a9274464563 100644 --- a/glib/src/lib.rs +++ b/glib/src/lib.rs @@ -171,7 +171,6 @@ mod date_time; mod time_span; mod time_zone; pub use self::time_span::*; -mod type_module; pub mod value; pub mod variant; mod variant_dict; diff --git a/glib/src/prelude.rs b/glib/src/prelude.rs index 86f1e4ba5b34..e9b8d388a1b9 100644 --- a/glib/src/prelude.rs +++ b/glib/src/prelude.rs @@ -6,7 +6,6 @@ pub use crate::{ gobject::traits::{TypeModuleExt, TypePluginExt}, param_spec::ParamSpecBuilderExt, - type_module::TypeModuleExtManual, Cast, CastNone, IsA, ObjectExt, ObjectType, ParamSpecType, StaticType, StaticTypeExt, StaticVariantType, ToSendValue, ToValue, ToVariant, }; diff --git a/glib/src/subclass/type_module.rs b/glib/src/subclass/type_module.rs index b04ca6ec6b92..e6ff35088695 100644 --- a/glib/src/subclass/type_module.rs +++ b/glib/src/subclass/type_module.rs @@ -78,6 +78,17 @@ unsafe extern "C" fn load( let imp = instance.imp(); let res = imp.load(); + // GLib type system expects a module to never be disposed if types has been + // successfully loaded. + // The following code prevents the Rust wrapper (`glib::TypeModule` subclass) + // to dispose the module when dropped by ensuring the reference count is > 1. + // Nothing is done if loading types has failed, allowing application to drop + // and dispose the invalid module. + if res && (*(type_module as *const gobject_ffi::GObject)).ref_count == 1 { + unsafe { + gobject_ffi::g_object_ref(type_module as _); + } + } res.into_glib() } @@ -127,7 +138,7 @@ mod tests { #[test] fn test_simple_type_module() { - let simple_type_module = SimpleTypeModule::new(); + let simple_type_module = crate::Object::new::(); // simulate the glib type system to load the module assert!(simple_type_module.use_()); simple_type_module.unuse(); diff --git a/glib/src/type_module.rs b/glib/src/type_module.rs deleted file mode 100644 index e675bbda08ee..000000000000 --- a/glib/src/type_module.rs +++ /dev/null @@ -1,34 +0,0 @@ -// Take a look at the license at the top of the repository in the LICENSE file. - -use crate::{ - object::IsClass, - translate::{Borrowed, FromGlibPtrBorrow}, - IsA, Object, TypeModule, -}; - -mod sealed { - pub trait Sealed {} - impl> Sealed for T {} -} - -pub trait TypeModuleExtManual: sealed::Sealed + IsA { - // rustdoc-stripper-ignore-next - /// Create a new instance of a type module object. - /// - /// Similar to [`Object::new`] but transfers ownership to GLib. This returns a - /// borrowed object, which does not drop the wrapped type module object when - /// going out of scope, and therefore preserves the reference count. - /// This ensures the module type is never disposed (as expected by GLib). - fn new() -> Borrowed - where - Self: IsClass + IsA + FromGlibPtrBorrow<*mut Self::GlibType>, - { - let module = Object::new::(); - unsafe { - let module_ptr = Self::into_glib_ptr(module); - Self::from_glib_borrow(module_ptr) - } - } -} - -impl> TypeModuleExtManual for O {}