Skip to content

Commit 68dd2e5

Browse files
committed
fix concurrency issues in macro helper attribute 'object_subclass_dynamic'
Signed-off-by: fbrouille <[email protected]>
1 parent 279063e commit 68dd2e5

File tree

2 files changed

+54
-34
lines changed

2 files changed

+54
-34
lines changed

glib-macros/src/enum_derive.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@ use heck::{ToKebabCase, ToShoutySnakeCase, ToUpperCamelCase};
44
use proc_macro2::TokenStream;
55
use proc_macro_error::abort_call_site;
66
use quote::{format_ident, quote, quote_spanned, ToTokens};
7-
use syn::{
8-
parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, Data, ExprArray, Ident,
9-
Variant,
10-
};
117

128
use crate::utils::{
139
crate_ident_new, gen_enum_from_glib, parse_nested_meta_items, parse_optional_nested_meta_items,
@@ -21,8 +17,8 @@ use crate::utils::{
2117
// value_nick: "goat\0" as *const _ as *const _,
2218
// },
2319
fn gen_enum_values(
24-
enum_name: &Ident,
25-
enum_variants: &Punctuated<Variant, Comma>,
20+
enum_name: &syn::Ident,
21+
enum_variants: &syn::punctuated::Punctuated<syn::Variant, syn::token::Comma>,
2622
) -> (TokenStream, usize) {
2723
let crate_ident = crate_ident_new();
2824

@@ -50,7 +46,7 @@ fn gen_enum_values(
5046

5147
n += 1;
5248
// generates a glib::gobject_ffi::GEnumValue.
53-
quote_spanned! {v.span()=>
49+
quote_spanned! {syn::spanned::Spanned::span(&v)=>
5450
#crate_ident::gobject_ffi::GEnumValue {
5551
value: #enum_name::#name as i32,
5652
value_name: #value_name as *const _ as *const _,
@@ -70,7 +66,7 @@ pub fn impl_enum(input: &syn::DeriveInput) -> TokenStream {
7066
let name = &input.ident;
7167

7268
let enum_variants = match input.data {
73-
Data::Enum(ref e) => &e.variants,
69+
syn::Data::Enum(ref e) => &e.variants,
7470
_ => abort_call_site!("#[derive(glib::Enum)] only supports enums"),
7571
};
7672
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 {
227223
// Registers the enum as a static type.
228224
fn register_enum_as_static(
229225
crate_ident: &TokenStream,
230-
name: &Ident,
226+
name: &syn::Ident,
231227
gtype_name: syn::LitStr,
232228
g_enum_values: TokenStream,
233229
nb_enum_values: usize,
@@ -273,15 +269,15 @@ fn register_enum_as_dynamic(
273269
crate_ident: &TokenStream,
274270
plugin_ty: TokenStream,
275271
lazy_registration: bool,
276-
name: &Ident,
272+
name: &syn::Ident,
277273
gtype_name: syn::LitStr,
278274
g_enum_values: TokenStream,
279275
nb_enum_values: usize,
280276
) -> TokenStream {
281277
// Wrap each GEnumValue to EnumValue
282-
let g_enum_values_expr: ExprArray = parse_quote! { [#g_enum_values] };
278+
let g_enum_values_expr: syn::ExprArray = syn::parse_quote! { [#g_enum_values] };
283279
let enum_values_iter = g_enum_values_expr.elems.iter().map(|v| {
284-
quote_spanned! {v.span()=>
280+
quote_spanned! {syn::spanned::Spanned::span(&v)=>
285281
#crate_ident::EnumValue::unsafe_from(#v),
286282
}
287283
});
@@ -310,6 +306,7 @@ fn register_enum_as_dynamic(
310306
);
311307
// name of the static array to store the enumeration values.
312308
let enum_values_array = format_ident!("{}_VALUES", name.to_string().to_shouty_snake_case());
309+
313310
quote! {
314311
/// The registration status type: a tuple of the weak reference on the plugin and of the GLib type.
315312
struct #registration_status_type(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type);
@@ -392,6 +389,7 @@ fn register_enum_as_dynamic(
392389

393390
// name of the static variable to store the GLib type.
394391
let gtype_status = format_ident!("{}_G_TYPE", name.to_string().to_shouty_snake_case());
392+
395393
quote! {
396394
/// The GLib type which can be safely shared between threads.
397395
static #gtype_status: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(#crate_ident::gobject_ffi::G_TYPE_INVALID);

glib-macros/src/object_subclass_attribute.rs

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
// Take a look at the license at the top of the repository in the LICENSE file.
22

3+
use heck::ToShoutySnakeCase;
34
use proc_macro2::TokenStream;
45
use proc_macro_error::abort_call_site;
5-
use quote::{quote, ToTokens};
6+
use quote::{format_ident, quote, ToTokens};
67

78
use crate::utils::{parse_optional_nested_meta_items, NestedMetaItem};
89

@@ -20,6 +21,18 @@ pub fn impl_object_subclass(input: &mut syn::ItemImpl) -> TokenStream {
2021
..
2122
} = input;
2223

24+
let self_ty_as_ident = match &**self_ty {
25+
syn::Type::Path(syn::TypePath { path, .. }) => path.require_ident(),
26+
_ => Err(syn::Error::new(
27+
syn::spanned::Spanned::span(self_ty),
28+
"expected this path to be an identifier",
29+
)),
30+
};
31+
let self_ty = match self_ty_as_ident {
32+
Ok(ident) => ident,
33+
Err(e) => return e.to_compile_error(),
34+
};
35+
2336
let mut plugin_type = NestedMetaItem::<syn::Path>::new("plugin_type").value_required();
2437
let mut lazy_registration =
2538
NestedMetaItem::<syn::LitBool>::new("lazy_registration").value_required();
@@ -190,7 +203,7 @@ pub fn impl_object_subclass(input: &mut syn::ItemImpl) -> TokenStream {
190203
// Registers the object subclass as a static type.
191204
fn register_object_subclass_as_static(
192205
crate_ident: &TokenStream,
193-
self_ty: &syn::Type,
206+
self_ty: &syn::Ident,
194207
) -> TokenStream {
195208
// registers the object subclass on first use (lazy registration).
196209
quote! {
@@ -212,7 +225,7 @@ fn register_object_subclass_as_static(
212225
// An object subclass can be reregistered as a dynamic type.
213226
fn register_object_subclass_as_dynamic(
214227
crate_ident: &TokenStream,
215-
self_ty: &syn::Type,
228+
self_ty: &syn::Ident,
216229
plugin_ty: TokenStream,
217230
lazy_registration: bool,
218231
) -> TokenStream {
@@ -222,27 +235,35 @@ fn register_object_subclass_as_dynamic(
222235
// registers the object subclass as a dynamic type on the first use (lazy registration).
223236
// a weak reference on the plugin is stored and will be used later on the first use of the object subclass.
224237
// this implementation relies on a static storage of a weak reference on the plugin and of the GLib type to know if the object subclass has been registered.
238+
239+
// the registration status type.
240+
let registration_status_type = format_ident!("{}RegistrationStatus", self_ty);
241+
// name of the static variable to store the registration status.
242+
let registration_status = format_ident!(
243+
"{}",
244+
registration_status_type.to_string().to_shouty_snake_case()
245+
);
246+
225247
quote! {
226-
impl #self_ty {
227-
/// Returns a mutable reference to the registration status: a tuple of the weak reference on the plugin and of the GLib type.
228-
/// This is safe because the mutable reference guarantees that no other threads are concurrently accessing the data.
229-
#[inline]
230-
fn get_registration_status_ref_mut() -> &'static mut Option<(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type)> {
231-
static mut REGISTRATION_STATUS: ::std::sync::Mutex<Option<(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type)>> = ::std::sync::Mutex::new(None);
232-
unsafe { REGISTRATION_STATUS.get_mut().unwrap() }
233-
}
248+
/// The registration status type: a tuple of the weak reference on the plugin and of the GLib type.
249+
struct #registration_status_type(<#plugin_ty as #crate_ident::clone::Downgrade>::Weak, #crate_ident::Type);
250+
unsafe impl Send for #registration_status_type {}
234251

252+
/// The registration status protected by a mutex guarantees so that no other threads are concurrently accessing the data.
253+
static #registration_status: ::std::sync::Mutex<Option<#registration_status_type>> = ::std::sync::Mutex::new(None);
254+
255+
impl #self_ty {
235256
/// Registers the object subclass as a dynamic type within the plugin only once.
236257
/// Plugin must have been used at least once.
237258
/// Do nothing if plugin has never been used or if the object subclass is already registered as a dynamic type.
238259
#[inline]
239260
fn register_type() {
240-
let registration_status_ref_mut = Self::get_registration_status_ref_mut();
241-
match registration_status_ref_mut {
261+
let mut registration_status = #registration_status.lock().unwrap();
262+
match ::std::ops::DerefMut::deref_mut(&mut registration_status) {
242263
// plugin has never been used, so the object subclass cannot be registered as a dynamic type.
243264
None => (),
244265
// plugin has been used and the object subclass has not been registered yet, so registers it as a dynamic type.
245-
Some((type_plugin, type_)) if !type_.is_valid() => {
266+
Some(#registration_status_type(type_plugin, type_)) if !type_.is_valid() => {
246267
*type_ = #crate_ident::subclass::register_dynamic_type::<#plugin_ty, Self>(&(type_plugin.upgrade().unwrap()));
247268
},
248269
// plugin has been used and the object subclass has already been registered as a dynamic type.
@@ -257,15 +278,15 @@ fn register_object_subclass_as_dynamic(
257278
/// If plugin is reused (and has reloaded the implementation) and the object subclass has not been registered yet as a dynamic type, do nothing.
258279
#[inline]
259280
pub fn on_implementation_load(type_plugin: &#plugin_ty) -> bool {
260-
let registration_status_ref_mut = Self::get_registration_status_ref_mut();
261-
match registration_status_ref_mut {
281+
let mut registration_status = #registration_status.lock().unwrap();
282+
match ::std::ops::DerefMut::deref_mut(&mut registration_status) {
262283
// plugin has never been used (this is the first time), so postpones registration of the object subclass as a dynamic type on the first use.
263284
None => {
264-
*registration_status_ref_mut = Some((#crate_ident::clone::Downgrade::downgrade(type_plugin), #crate_ident::Type::INVALID));
285+
*registration_status = Some(#registration_status_type(#crate_ident::clone::Downgrade::downgrade(type_plugin), #crate_ident::Type::INVALID));
265286
true
266287
},
267288
// plugin has been used at least one time and the object subclass has been registered as a dynamic type at least one time, so re-registers it.
268-
Some((_, type_)) if type_.is_valid() => {
289+
Some(#registration_status_type(_, type_)) if type_.is_valid() => {
269290
*type_ = #crate_ident::subclass::register_dynamic_type::<#plugin_ty, Self>(type_plugin);
270291
type_.is_valid()
271292
},
@@ -281,15 +302,15 @@ fn register_object_subclass_as_dynamic(
281302
/// Else do nothing.
282303
#[inline]
283304
pub fn on_implementation_unload(type_plugin_: &#plugin_ty) -> bool {
284-
let registration_status_ref_mut = Self::get_registration_status_ref_mut();
285-
match registration_status_ref_mut {
305+
let mut registration_status = #registration_status.lock().unwrap();
306+
match ::std::ops::DerefMut::deref_mut(&mut registration_status) {
286307
// plugin has never been used, so unload implementation is unexpected.
287308
None => false,
288309
// plugin has been used at least one time and the object subclass has been registered as a dynamic type at least one time.
289-
Some((_, type_)) if type_.is_valid() => true,
310+
Some(#registration_status_type(_, type_)) if type_.is_valid() => true,
290311
// plugin has been used at least one time but the object subclass has not been registered yet as a dynamic type, so cancels the postponed registration.
291312
Some(_) => {
292-
*registration_status_ref_mut = None;
313+
*registration_status = None;
293314
true
294315
}
295316
}
@@ -298,6 +319,7 @@ fn register_object_subclass_as_dynamic(
298319
}
299320
} else {
300321
// registers immediately the object subclass as a dynamic type.
322+
301323
quote! {
302324
impl #self_ty {
303325
/// Do nothing as the object subclass has been registered on implementation load.

0 commit comments

Comments
 (0)