From 8a11f3d3aeb2700ab0f90c832c20e0d230f07d88 Mon Sep 17 00:00:00 2001 From: evdokimovs <49490279+evdokimovs@users.noreply.github.com> Date: Thu, 26 Dec 2024 13:11:49 -0300 Subject: [PATCH] Remove `static mut` usage in `medea-macro` crate's expansions (#196) Additionally: - fix unused import related to feature gated Android logger --- Cargo.lock | 7 ++ Cargo.toml | 7 +- crates/medea-macro/src/dart_bridge.rs | 102 ++++++++++++++++---------- proto/client-api/src/lib.rs | 10 ++- src/platform/dart/mod.rs | 7 +- 5 files changed, 87 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec807a02..889d4964 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2332,6 +2332,7 @@ dependencies = [ "serde", "serde_json", "simple_logger", + "sync-unsafe-cell", "talc", "tracerr", "url", @@ -3470,6 +3471,12 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "sync-unsafe-cell" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8deaecba5382c095cb432cd1e21068dadb144208f057b13720e76bf89749beb4" + [[package]] name = "sync_wrapper" version = "0.1.2" diff --git a/Cargo.toml b/Cargo.toml index 1467fbdb..ae686424 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,8 @@ readme = "README.md" keywords = ["medea", "jason", "webrtc", "client", "browser"] categories = ["multimedia", "api-bindings", "web-programming", "wasm"] include = ["/src/", "/build.rs", "/CHANGELOG.md", "/LICENSE.md"] -# TODO: Remove when https://github.com/rustwasm/wasm-pack/issues/1441 will be resolved. +# TODO: Remove once rustwasm/wasm-pack#1441 is resolved: +# https://github.com/rustwasm/wasm-pack/issues/1441 wasm-opt = false [workspace] @@ -69,6 +70,10 @@ dart-sys = "4.1" flutter_rust_bridge = { version = "=2.7.0", features = ["anyhow", "dart-opaque", "rust-async"], default-features = false } libc = "0.2" send_wrapper = "0.6" +# TODO: Replace `sync_unsafe_cell` with `std` once the following API is +# stabilized: +# https://doc.rust-lang.org/std/cell/struct.SyncUnsafeCell.html +sync-unsafe-cell = "0.1" [target.'cfg(target_family = "wasm")'.dependencies] backoff = { version = "0.4", features = ["wasm-bindgen"] } diff --git a/crates/medea-macro/src/dart_bridge.rs b/crates/medea-macro/src/dart_bridge.rs index 3b2cc97a..ea639f70 100644 --- a/crates/medea-macro/src/dart_bridge.rs +++ b/crates/medea-macro/src/dart_bridge.rs @@ -133,8 +133,8 @@ impl ModExpander { let type_aliases = self.fn_expanders.iter().map(FnExpander::gen_fn_type); - let static_muts = - self.fn_expanders.iter().map(FnExpander::gen_fn_static_mut); + let fn_storages = + self.fn_expanders.iter().map(FnExpander::gen_fn_storages); let register_fn_name = &self.register_fn_name; let register_fn_inputs = self @@ -162,7 +162,7 @@ impl ModExpander { #( #type_aliases )* - #( #static_muts )* + #( #fn_storages )* #( #errors_slots )* @@ -316,10 +316,10 @@ impl<'a> IdentGenerator<'a> { ) } - /// Returns a [`syn::Ident`] for the [`FnExpander`]'s `static mut`. + /// Returns a [`syn::Ident`] for the [`FnExpander`]'s storage. /// /// Generates something like `PEER_CONNECTION__CREATE_OFFER__FUNCTION` - fn static_mut(&self) -> syn::Ident { + fn fn_storage(&self) -> syn::Ident { format_ident!( "{}__{}__FUNCTION", self.prefix.to_string().to_screaming_snake_case(), @@ -363,11 +363,11 @@ struct FnExpander { /// [`syn::Ident`] of the type alias for the extern Dart function pointer. type_alias_ident: syn::Ident, - /// [`syn::Ident`] of the `static mut` storing extern Dart function - /// pointer. - static_mut_ident: syn::Ident, + /// [`syn::Ident`] of the storage storing extern Dart function pointer. + fn_storage_ident: syn::Ident, - /// [`syn::Ident`] of the function error slot (`static mut Option`). + /// [`syn::Ident`] of the function error slot + /// (`thread_local! { RefCell> }`). error_slot_ident: syn::Ident, /// [`syn::Ident`] of the extern function that saves error in its slot. @@ -455,7 +455,7 @@ impl FnExpander { let ident_generator = IdentGenerator::new(prefix, &item.sig.ident); Ok(Self { type_alias_ident: ident_generator.type_alias(), - static_mut_ident: ident_generator.static_mut(), + fn_storage_ident: ident_generator.fn_storage(), error_slot_ident: ident_generator.error_slot_name(), error_setter_ident: ident_generator.error_setter_name(), ident: item.sig.ident, @@ -502,18 +502,23 @@ impl FnExpander { /// # Example of the generated code /// /// ```ignore - /// PEER_CONNECTION__CREATE_OFFER__FUNCTION.write(create_offer) + /// SyncUnsafeCell::get(&PEER_CONNECTION__CREATE_OFFER__FUNCTION) + /// = Some(create_offer) /// ``` fn gen_register_fn_expr(&self) -> syn::Expr { - let fn_static_mut = &self.static_mut_ident; + let fn_storage_ident = &self.fn_storage_ident; let ident = &self.ident; + // TODO: Replace `sync_unsafe_cell` with `std` once the + // following API is stabilized: + // https://doc.rust-lang.org/std/cell/struct.SyncUnsafeCell.html parse_quote! { - #fn_static_mut.write(#ident) + *::sync_unsafe_cell::SyncUnsafeCell::get(&#fn_storage_ident) + = Some(#ident) } } - /// Generates type alias of the extern Dart function. + /// Generates a type alias of an extern Dart function. /// /// # Example of the generated code /// @@ -531,38 +536,46 @@ impl FnExpander { } } - /// Generates `static mut` for the extern Dart function pointer storing. + /// Generates a storage for extern Dart function pointer storing. /// /// # Example of generated code /// /// ```ignore - /// static mut PEER_CONNECTION__CREATE_OFFER__FUNCTION: - /// std::mem::MaybeUninit = - /// std::mem::MaybeUninit::uninit(); + /// static PEER_CONNECTION__CREATE_OFFER__FUNCTION: + /// ::sync_unsafe_cell::SyncUnsafeCell< + /// Option + /// > = ::sync_unsafe_cell::SyncUnsafeCell::new(None); /// ``` - fn gen_fn_static_mut(&self) -> TokenStream { - let name = &self.static_mut_ident; + fn gen_fn_storages(&self) -> TokenStream { + let name = &self.fn_storage_ident; let type_alias = &self.type_alias_ident; + // TODO: Replace `sync_unsafe_cell` with `std` once the + // following API is stabilized: + // https://doc.rust-lang.org/std/cell/struct.SyncUnsafeCell.html quote! { - static mut #name: ::std::mem::MaybeUninit<#type_alias> = - ::std::mem::MaybeUninit::uninit(); + static #name: ::sync_unsafe_cell::SyncUnsafeCell< + Option<#type_alias> + > = ::sync_unsafe_cell::SyncUnsafeCell::new(None); } } - /// Generates Dart function caller for Rust. + /// Generates a Dart function caller for Rust. /// /// # Example of generated code /// /// ```ignore - /// pub unsafe fn create_offer( - /// peer: Dart_Handle, - /// ) -> Result { - /// let res = - /// (PEER_CONNECTION__CREATE_OFFER__FUNCTION.assume_init_ref())(peer); - /// - /// if let Some(e) = PEER_CONNECTION__CREATE_OFFER__ERROR.take() - /// { Err(e) } else { Ok(res) } + /// pub unsafe fn #name(#args) #ret_ty { + /// let res = ( + /// *(*#fn_storage_ident.get()) + /// .as_ref() + /// .unwrap() + /// )(#( #args_idents ),*); + /// if let Some(e) = #error_slot.take() { + /// Err(e) + /// } else { + /// Ok(res) + /// } /// } /// ``` fn gen_caller_fn(&self) -> TokenStream { @@ -582,12 +595,16 @@ impl FnExpander { let ret_ty = &self.ret_ty; - let static_mut = &self.static_mut_ident; + let fn_storage_ident = &self.fn_storage_ident; quote! { #( #doc_attrs )* pub unsafe fn #name(#args) #ret_ty { - let res = (#static_mut.assume_init_ref())(#( #args_idents ),*); + let res = ( + *(*#fn_storage_ident.get()) + .as_ref() + .unwrap() + )(#( #args_idents ),*); if let Some(e) = #error_slot.take() { Err(e) } else { @@ -602,13 +619,20 @@ impl FnExpander { /// # Example of generated code /// /// ```ignore - /// static mut PEER_CONNECTION__CREATE_OFFER__ERROR: Option = None; + /// ::std::thread_local! { + /// static PEER_CONNECTION__CREATE_OFFER__ERROR: ::std::cell::RefCell< + /// Option, + /// > = ::std::cell::RefCell::default(); + /// } /// ``` fn get_errors_slot(&self) -> TokenStream { let name = &self.error_slot_ident; quote! { - static mut #name: Option = None; + ::std::thread_local! { + static #name: ::std::cell::RefCell> = + ::std::cell::RefCell::new(None); + } } } @@ -619,10 +643,10 @@ impl FnExpander { /// ```ignore /// #[no_mangle] /// pub unsafe extern "C" fn peer_connection__connection_state__set_error( - /// err: Dart_Handle + /// err: Dart_Handle, /// ) { - /// PEER_CONNECTION__CONNECTION_STATE__ERROR = - /// Some(Error::from_handle(err)); + /// PEER_CONNECTION__CONNECTION_STATE__ERROR + /// .set(Some(Error::from_handle(err))); /// } /// ``` fn gen_error_setter(&self) -> TokenStream { @@ -634,7 +658,7 @@ impl FnExpander { #[doc = #doc] #[no_mangle] pub unsafe extern "C" fn #fn_name(err: Dart_Handle) { - #error_slot = Some(Error::from_handle(err)); + #error_slot.set(Some(Error::from_handle(err))); } } } diff --git a/proto/client-api/src/lib.rs b/proto/client-api/src/lib.rs index 82a78a54..50c98cf3 100644 --- a/proto/client-api/src/lib.rs +++ b/proto/client-api/src/lib.rs @@ -319,7 +319,10 @@ impl_incrementable!(TrackId); /// Message sent by Media Server to Web Client. #[cfg_attr( - target_family = "wasm", + any( + target_family = "wasm", + all(target_arch = "arm", target_os = "android") + ), expect(variant_size_differences, reason = "`Event` is the most common") )] #[derive(Clone, Debug, Eq, PartialEq)] @@ -348,7 +351,10 @@ pub enum ServerMsg { /// Message by Web Client to Media Server. #[cfg_attr( - target_family = "wasm", + any( + target_family = "wasm", + all(target_arch = "arm", target_os = "android") + ), expect(variant_size_differences, reason = "`Command` is the most common") )] #[derive(Clone, Debug, PartialEq)] diff --git a/src/platform/dart/mod.rs b/src/platform/dart/mod.rs index e1e3f5e1..cc71900f 100644 --- a/src/platform/dart/mod.rs +++ b/src/platform/dart/mod.rs @@ -30,10 +30,7 @@ pub mod transceiver; pub mod transport; pub mod utils; -use std::{ - cell::{LazyCell, RefCell}, - panic, -}; +use std::{cell::RefCell, panic}; use libc::c_void; @@ -109,6 +106,8 @@ pub fn init_logger() { /// Initializes [`simple_logger`] as the default application logger with filter /// level set to [`log::LevelFilter::Debug`]. pub fn init_logger() { + use std::cell::LazyCell; + thread_local! { /// [`LazyCell`] ensuring that a [`simple_logger`] is initialized only /// once.