Merge pull request #1256 from fbrouille/fix_concurrency_issues

Fix concurrency issues

Author: sdroege

glib-macros/src/enum_derive.rs

@@ -4,10 +4,6 @@ use heck::{ToKebabCase, ToShoutySnakeCase, ToUpperCamelCase};
 use proc_macro2::TokenStream;
 use proc_macro_error::abort_call_site;
 use quote::{format_ident, quote, quote_spanned, ToTokens};
-use syn::{
-    parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, Data, ExprArray, Ident,
-    Variant,
-};
 
 use crate::utils::{
     crate_ident_new, gen_enum_from_glib, parse_nested_meta_items, parse_optional_nested_meta_items,
@@ -21,8 +17,8 @@ use crate::utils::{
 //         value_nick: "goat\0" as *const _ as *const _,
 //     },
 fn gen_enum_values(
-    enum_name: &Ident,
-    enum_variants: &Punctuated<Variant, Comma>,
+    enum_name: &syn::Ident,
+    enum_variants: &syn::punctuated::Punctuated<syn::Variant, syn::token::Comma>,
 ) -> (TokenStream, usize) {
     let crate_ident = crate_ident_new();
 
@@ -50,7 +46,7 @@ fn gen_enum_values(
 
         n += 1;
         // generates a glib::gobject_ffi::GEnumValue.
-        quote_spanned! {v.span()=>
+        quote_spanned! {syn::spanned::Spanned::span(&v)=>
             #crate_ident::gobject_ffi::GEnumValue {
                 value: #enum_name::#name as i32,
                 value_name: #value_name as *const _ as *const _,
@@ -70,7 +66,7 @@ pub fn impl_enum(input: &syn::DeriveInput) -> TokenStream {
     let name = &input.ident;
 
     let enum_variants = match input.data {
-        Data::Enum(ref e) => &e.variants,
+        syn::Data::Enum(ref e) => &e.variants,
         _ => abort_call_site!("#[derive(glib::Enum)] only supports enums"),
     };
     let (g_enum_values, nb_enum_values) = gen_enum_values(name, enum_variants);
@@ -227,7 +223,7 @@ pub fn impl_enum(input: &syn::DeriveInput) -> TokenStream {
 // Registers the enum as a static type.
 fn register_enum_as_static(
     crate_ident: &TokenStream,
-    name: &Ident,
+    name: &syn::Ident,
     gtype_name: syn::LitStr,
     g_enum_values: TokenStream,
     nb_enum_values: usize,
@@ -273,15 +269,15 @@ fn register_enum_as_dynamic(
     crate_ident: &TokenStream,
     plugin_ty: TokenStream,
     lazy_registration: bool,
-    name: &Ident,
+    name: &syn::Ident,
     gtype_name: syn::LitStr,
     g_enum_values: TokenStream,
     nb_enum_values: usize,
 ) -> TokenStream {
     // Wrap each GEnumValue to EnumValue
-    let g_enum_values_expr: ExprArray = parse_quote! { [#g_enum_values] };
+    let g_enum_values_expr: syn::ExprArray = syn::parse_quote! { [#g_enum_values] };
     let enum_values_iter = g_enum_values_expr.elems.iter().map(|v| {
-        quote_spanned! {v.span()=>
+        quote_spanned! {syn::spanned::Spanned::span(&v)=>
             #crate_ident::EnumValue::unsafe_from(#v),
         }
     });
@@ -310,6 +306,7 @@ fn register_enum_as_dynamic(
         );
         // name of the static array to store the enumeration values.
         let enum_values_array = format_ident!("{}_VALUES", name.to_string().to_shouty_snake_case());
+
         quote! {
             /// The registration status type: a tuple of the weak reference on the plugin and of the GLib type.
             struct #registration_status_type(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type);
@@ -392,6 +389,7 @@ fn register_enum_as_dynamic(
 
         // name of the static variable to store the GLib type.
         let gtype_status = format_ident!("{}_G_TYPE", name.to_string().to_shouty_snake_case());
+
         quote! {
             /// The GLib type which can be safely shared between threads.
             static #gtype_status: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(#crate_ident::gobject_ffi::G_TYPE_INVALID);
@@ -400,7 +398,7 @@ fn register_enum_as_dynamic(
                 /// Do nothing as the enum has been registered on implementation load.
                 #[inline]
                 fn register_enum() -> #crate_ident::Type {
-                    let gtype = #gtype_status.load(::std::sync::atomic::Ordering::Relaxed);
+                    let gtype = #gtype_status.load(::std::sync::atomic::Ordering::Acquire);
                     unsafe { <#crate_ident::Type as #crate_ident::translate::FromGlib<#crate_ident::ffi::GType>>::from_glib(gtype) }
                 }
 
@@ -410,7 +408,7 @@ fn register_enum_as_dynamic(
                 pub fn on_implementation_load(type_plugin: &#plugin_ty) -> bool {
                     static VALUES: #enum_values;
                     let gtype = #crate_ident::translate::IntoGlib::into_glib(<#plugin_ty as glib::prelude::DynamicObjectRegisterExt>::register_dynamic_enum(type_plugin, #gtype_name, VALUES.as_ref()));
-                    #gtype_status.store(gtype, ::std::sync::atomic::Ordering::Relaxed);
+                    #gtype_status.store(gtype, ::std::sync::atomic::Ordering::Release);
                     gtype != #crate_ident::gobject_ffi::G_TYPE_INVALID
                 }
 

glib-macros/src/object_interface_attribute.rs

@@ -1,8 +1,9 @@
 // Take a look at the license at the top of the repository in the LICENSE file.
 
+use heck::ToShoutySnakeCase;
 use proc_macro2::TokenStream;
 use proc_macro_error::abort_call_site;
-use quote::{quote, ToTokens};
+use quote::{format_ident, quote, ToTokens};
 
 use crate::utils::{parse_optional_nested_meta_items, NestedMetaItem};
 
@@ -21,6 +22,18 @@ pub fn impl_object_interface(input: &mut syn::ItemImpl) -> TokenStream {
         ..
     } = input;
 
+    let self_ty_as_ident = match &**self_ty {
+        syn::Type::Path(syn::TypePath { path, .. }) => path.require_ident(),
+        _ => Err(syn::Error::new(
+            syn::spanned::Spanned::span(self_ty),
+            "expected this path to be an identifier",
+        )),
+    };
+    let self_ty = match self_ty_as_ident {
+        Ok(ident) => ident,
+        Err(e) => return e.to_compile_error(),
+    };
+
     let mut plugin_type = NestedMetaItem::<syn::Path>::new("plugin_type").value_required();
     let mut lazy_registration =
         NestedMetaItem::<syn::LitBool>::new("lazy_registration").value_required();
@@ -95,7 +108,7 @@ pub fn impl_object_interface(input: &mut syn::ItemImpl) -> TokenStream {
 // Registers the object interface as a static type.
 fn register_object_interface_as_static(
     crate_ident: &TokenStream,
-    self_ty: &syn::Type,
+    self_ty: &syn::Ident,
 ) -> TokenStream {
     // registers the interface on first use (lazy registration).
     quote! {
@@ -122,7 +135,7 @@ fn register_object_interface_as_static(
 // An object interface can be reregistered as a dynamic type.
 fn register_object_interface_as_dynamic(
     crate_ident: &TokenStream,
-    self_ty: &syn::Type,
+    self_ty: &syn::Ident,
     plugin_ty: TokenStream,
     lazy_registration: bool,
 ) -> TokenStream {
@@ -132,32 +145,40 @@ fn register_object_interface_as_dynamic(
         // registers the object interface as a dynamic type on the first use (lazy registration).
         // a weak reference on the plugin is stored and will be used later on the first use of the object interface.
         // this implementation relies on a static storage of a weak reference on the plugin and of the GLib type to know if the object interface has been registered.
+
+        // the registration status type.
+        let registration_status_type = format_ident!("{}RegistrationStatus", self_ty);
+        // name of the static variable to store the registration status.
+        let registration_status = format_ident!(
+            "{}",
+            registration_status_type.to_string().to_shouty_snake_case()
+        );
+
         quote! {
-            impl #self_ty {
-                /// Returns a mutable reference to the registration status: a tuple of the weak reference on the plugin and of the GLib type.
-                /// This is safe because the mutable reference guarantees that no other threads are concurrently accessing the data.
-                #[inline]
-                fn get_registration_status_ref_mut() -> &'static mut Option<(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type)> {
-                    static mut REGISTRATION_STATUS: ::std::sync::Mutex<Option<(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type)>> = ::std::sync::Mutex::new(None);
-                    unsafe { REGISTRATION_STATUS.get_mut().unwrap() }
-                }
+            /// The registration status type: a tuple of the weak reference on the plugin and of the GLib type.
+            struct #registration_status_type(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type);
+            unsafe impl Send for #registration_status_type {}
 
+            /// The registration status protected by a mutex guarantees so that no other threads are concurrently accessing the data.
+            static #registration_status: ::std::sync::Mutex<Option<#registration_status_type>> = ::std::sync::Mutex::new(None);
+
+            impl #self_ty {
                 /// Registers the object interface as a dynamic type within the plugin only once.
                 /// Plugin must have been used at least once.
                 /// Do nothing if plugin has never been used or if the object interface is already registered as a dynamic type.
                 #[inline]
                 fn register_interface() -> #crate_ident::Type {
-                    let registration_status_ref_mut = Self::get_registration_status_ref_mut();
-                    match registration_status_ref_mut {
+                    let mut registration_status = #registration_status.lock().unwrap();
+                    match ::std::ops::DerefMut::deref_mut(&mut registration_status) {
                         // plugin has never been used, so the object interface cannot be registered as a dynamic type.
                         None => #crate_ident::Type::INVALID,
                         // plugin has been used and the object interface has not been registered yet, so registers it as a dynamic type.
-                        Some((type_plugin, type_)) if !type_.is_valid() => {
+                        Some(#registration_status_type(type_plugin, type_)) if !type_.is_valid() => {
                             *type_ = #crate_ident::subclass::register_dynamic_interface::<#plugin_ty, Self>(&(type_plugin.upgrade().unwrap()));
                             *type_
                         },
                         // plugin has been used and the object interface has already been registered as a dynamic type.
-                        Some((_, type_)) => *type_
+                        Some(#registration_status_type(_, type_)) => *type_
                     }
                 }
 
@@ -168,15 +189,15 @@ fn register_object_interface_as_dynamic(
                 /// If plugin is reused (and has reloaded the implementation) and the object interface has not been registered yet as a dynamic type, do nothing.
                 #[inline]
                 pub fn on_implementation_load(type_plugin: &#plugin_ty) -> bool {
-                    let registration_status_ref_mut = Self::get_registration_status_ref_mut();
-                    match registration_status_ref_mut {
+                    let mut registration_status = #registration_status.lock().unwrap();
+                    match ::std::ops::DerefMut::deref_mut(&mut registration_status) {
                         // plugin has never been used (this is the first time), so postpones registration of the object interface as a dynamic type on the first use.
                         None => {
-                            *registration_status_ref_mut = Some((#crate_ident::clone::Downgrade::downgrade(type_plugin), #crate_ident::Type::INVALID));
+                            *registration_status = Some(#registration_status_type(#crate_ident::clone::Downgrade::downgrade(type_plugin), #crate_ident::Type::INVALID));
                             true
                         },
                         // plugin has been used at least one time and the object interface has been registered as a dynamic type at least one time, so re-registers it.
-                        Some((_, type_)) if type_.is_valid() => {
+                        Some(#registration_status_type(_, type_)) if type_.is_valid() => {
                             *type_ = #crate_ident::subclass::register_dynamic_interface::<#plugin_ty, Self>(type_plugin);
                             type_.is_valid()
                         },
@@ -192,15 +213,15 @@ fn register_object_interface_as_dynamic(
                 /// Else do nothing.
                 #[inline]
                 pub fn on_implementation_unload(type_plugin_: &#plugin_ty) -> bool {
-                    let registration_status_ref_mut = Self::get_registration_status_ref_mut();
-                    match registration_status_ref_mut {
+                    let mut registration_status = #registration_status.lock().unwrap();
+                    match ::std::ops::DerefMut::deref_mut(&mut registration_status) {
                         // plugin has never been used, so unload implementation is unexpected.
                         None => false,
                         // plugin has been used at least one time and the object interface has been registered as a dynamic type at least one time.
-                        Some((_, type_)) if type_.is_valid() => true,
+                        Some(#registration_status_type(_, type_)) if type_.is_valid() => true,
                         // plugin has been used at least one time but the object interface has not been registered yet as a dynamic type, so cancels the postponed registration.
                         Some(_) => {
-                            *registration_status_ref_mut = None;
+                            *registration_status = None;
                             true
                         }
                     }
@@ -209,29 +230,29 @@ fn register_object_interface_as_dynamic(
         }
     } else {
         // registers immediately the object interface as a dynamic type.
+
+        // name of the static variable to store the GLib type.
+        let gtype_status = format_ident!("{}_G_TYPE", self_ty.to_string().to_shouty_snake_case());
+
         quote! {
-            impl #self_ty {
-                /// Returns a mutable reference to the GLib type.
-                /// This is safe because the mutable reference guarantees that no other threads are concurrently accessing the atomic data.
-                #[inline]
-                fn get_type_mut() -> &'static mut #crate_ident::ffi::GType {
-                    static mut TYPE: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(#crate_ident::gobject_ffi::G_TYPE_INVALID);
-                    unsafe { TYPE.get_mut() }
-                }
+            /// The GLib type which can be safely shared between threads.
+            static #gtype_status: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(#crate_ident::gobject_ffi::G_TYPE_INVALID);
 
+            impl #self_ty {
                 /// Do nothing as the object interface has been registered on implementation load.
                 #[inline]
                 fn register_interface() -> #crate_ident::Type {
-                    unsafe { <#crate_ident::Type as #crate_ident::translate::FromGlib<#crate_ident::ffi::GType>>::from_glib(*Self::get_type_mut()) }
+                    let gtype = #gtype_status.load(::std::sync::atomic::Ordering::Acquire);
+                    unsafe { <#crate_ident::Type as #crate_ident::translate::FromGlib<#crate_ident::ffi::GType>>::from_glib(gtype) }
                 }
 
                 /// Registers the object interface as a dynamic type within the plugin.
                 /// The object interface can be registered several times as a dynamic type.
                 #[inline]
                 pub fn on_implementation_load(type_plugin: &#plugin_ty) -> bool {
-                    let type_mut = Self::get_type_mut();
-                    *type_mut = #crate_ident::translate::IntoGlib::into_glib(#crate_ident::subclass::register_dynamic_interface::<#plugin_ty, Self>(type_plugin));
-                    *type_mut != #crate_ident::gobject_ffi::G_TYPE_INVALID
+                    let gtype = #crate_ident::translate::IntoGlib::into_glib(#crate_ident::subclass::register_dynamic_interface::<#plugin_ty, Self>(type_plugin));
+                    #gtype_status.store(gtype, ::std::sync::atomic::Ordering::Release);
+                    gtype != #crate_ident::gobject_ffi::G_TYPE_INVALID
                 }
 
                 /// Do nothing as object interfaces registered as dynamic types are never unregistered.