Get rid of static mut usage (#193)

Additionally: - temporary disable `wasm-opt` (rustwasm/wasm-pack#1441) Co-authored-by: alexlapa <[email protected]> Co-authored-by: Kai Ren <[email protected]>
instrumentisto · Dec 19, 2024 · 02808ed · 02808ed
1 parent e90db43
commit 02808ed
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 64 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -13,6 +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.
+wasm-opt = false
 
 [workspace]
 members = [

diff --git a/crates/medea-macro/src/dart_bridge.rs b/crates/medea-macro/src/dart_bridge.rs
@@ -11,6 +11,7 @@ use syn::{parse_quote, punctuated::Punctuated, spanned::Spanned as _, token};
 #[cfg(feature = "dart-codegen")]
 use crate::dart_codegen::{DartCodegen, FnRegistrationBuilder};
 
+// TODO: Refactor to get rid of `static mut` in this macro.
 /// Expands `#[dart_bridge]` attribute placed on a Rust module declaration.
 pub(crate) fn expand(
     args: TokenStream,

diff --git a/src/api/dart/api/local_media_track.rs b/src/api/dart/api/local_media_track.rs
@@ -40,9 +40,12 @@ impl LocalMediaTrack {
     #[frb(sync)]
     #[must_use]
     pub fn get_track(&self) -> DartOpaque {
-        DartOpaque::new(self.0.get_track().handle() as _, unsafe {
-            DART_HANDLER_PORT.unwrap()
-        })
+        DartOpaque::new(
+            self.0.get_track().handle() as _,
+            DART_HANDLER_PORT
+                .get()
+                .expect("`DART_HANDLER_PORT` must be initialized"),
+        )
     }
 
     /// Returns a [`MediaKind::Audio`] if the provided [`LocalMediaTrack`]

diff --git a/src/api/dart/api/mod.rs b/src/api/dart/api/mod.rs
@@ -41,7 +41,7 @@ pub mod remote_media_track;
 pub mod room;
 pub mod room_close_reason;
 
-use std::ptr;
+use std::{cell::Cell, ptr};
 
 use flutter_rust_bridge::{frb, DartOpaque};
 
@@ -63,8 +63,10 @@ pub use self::{
     room::RoomHandle, room_close_reason::RoomCloseReason,
 };
 
-/// Used to create [`DartOpaque`]s on the Rust side.
-pub static mut DART_HANDLER_PORT: Option<i64> = None;
+thread_local! {
+    /// Used to create [`DartOpaque`]s on the Rust side.
+    pub static DART_HANDLER_PORT: Cell<Option<i64>> = Cell::default();
+}
 
 /// Rust structure having wrapper class in Dart.
 ///
@@ -381,9 +383,5 @@ pub fn on_panic(cb: DartOpaque) {
 #[frb(sync)]
 #[must_use]
 pub fn set_dart_opaque_message_port(dart_handler_port: i64) {
-    // TODO: Refactor to get rid of `static mut`.
-    #[expect(static_mut_refs, reason = "needs refactoring")]
-    unsafe {
-        DART_HANDLER_PORT.replace(dart_handler_port);
-    }
+    DART_HANDLER_PORT.set(Some(dart_handler_port));
 }
diff --git a/src/api/dart/api/remote_media_track.rs b/src/api/dart/api/remote_media_track.rs
@@ -34,9 +34,12 @@ impl RemoteMediaTrack {
     #[frb(sync)]
     #[must_use]
     pub fn get_track(&self) -> DartOpaque {
-        DartOpaque::new(self.0.get_track().handle() as _, unsafe {
-            DART_HANDLER_PORT.unwrap()
-        })
+        DartOpaque::new(
+            self.0.get_track().handle() as _,
+            DART_HANDLER_PORT
+                .get()
+                .expect("`DART_HANDLER_PORT` must be initialized"),
+        )
     }
 
     /// Sets callback to invoke once this [`RemoteMediaTrack`] is muted.

diff --git a/src/api/dart/err.rs b/src/api/dart/err.rs
@@ -135,12 +135,16 @@ impl DartError {
     }
 }
 
-#[expect(clippy::fallible_impl_from, reason = "FFI error is unexpected")]
 impl From<DartError> for DartOpaque {
     fn from(val: DartError) -> Self {
         let boxed = unsafe { Box::from_raw(val.0.as_ptr()) };
-        #[expect(clippy::unwrap_used, reason = "FFI error is unexpected")]
-        Self::new((*boxed).cast(), unsafe { DART_HANDLER_PORT.unwrap() })
+        #[expect(clippy::expect_used, reason = "intended behavior")]
+        Self::new(
+            (*boxed).cast(),
+            DART_HANDLER_PORT
+                .get()
+                .expect("`DART_HANDLER_PORT` must be initialized"),
+        )
     }
 }
 

diff --git a/src/platform/dart/executor/mod.rs b/src/platform/dart/executor/mod.rs
@@ -2,7 +2,12 @@
 
 mod task;
 
-use std::{future::Future, ptr, rc::Rc};
+use std::{
+    future::Future,
+    ptr,
+    rc::Rc,
+    sync::{atomic, atomic::AtomicI64},
+};
 
 use dart_sys::{
     Dart_CObject, Dart_CObject_Type_Dart_CObject_kInt64, Dart_Port,
@@ -18,12 +23,15 @@ pub fn spawn(fut: impl Future<Output = ()> + 'static) {
     Task::spawn(Box::pin(fut));
 }
 
-/// A [`Dart_Port`] used to send [`Task`]'s poll commands so Dart will poll Rust
+/// Atomic variant of a [`Dart_Port`].
+type AtomicDartPort = AtomicI64;
+
+/// [`Dart_Port`] used to send [`Task`]'s poll commands so Dart will poll Rust
 /// [`Future`]s.
 ///
 /// Must be initialized with the [`rust_executor_init()`] function during FFI
 /// initialization.
-static mut WAKE_PORT: Option<Dart_Port> = None;
+static WAKE_PORT: AtomicDartPort = AtomicI64::new(0);
 
 /// Initializes Dart-driven async [`Task`] executor.
 ///
@@ -35,9 +43,7 @@ static mut WAKE_PORT: Option<Dart_Port> = None;
 /// Must ONLY be called by Dart during FFI initialization.
 #[no_mangle]
 pub unsafe extern "C" fn rust_executor_init(wake_port: Dart_Port) {
-    unsafe {
-        WAKE_PORT = Some(wake_port);
-    }
+    WAKE_PORT.store(wake_port, atomic::Ordering::Release);
 }
 
 /// Polls the provided [`Task`].
@@ -58,7 +64,8 @@ pub unsafe extern "C" fn rust_executor_poll_task(task: ptr::NonNull<Task>) {
 /// [`WAKE_PORT`]. When received, Dart must poll it by calling the
 /// [`rust_executor_poll_task()`] function.
 fn task_wake(task: Rc<Task>) {
-    let wake_port = unsafe { WAKE_PORT }.unwrap();
+    let wake_port = WAKE_PORT.load(atomic::Ordering::Acquire);
+    debug_assert!(wake_port > 0, "`WAKE_PORT` address must be initialized");
     let task = Rc::into_raw(task);
 
     let mut task_addr = Dart_CObject {

diff --git a/src/platform/dart/mod.rs b/src/platform/dart/mod.rs
@@ -30,7 +30,10 @@ pub mod transceiver;
 pub mod transport;
 pub mod utils;
 
-use std::panic;
+use std::{
+    cell::{LazyCell, RefCell},
+    panic,
+};
 
 use libc::c_void;
 
@@ -66,32 +69,31 @@ pub unsafe extern "C" fn init_jason_dart_api_dl(data: *mut c_void) -> isize {
 /// Dart's functions.
 pub fn set_panic_hook() {
     panic::set_hook(Box::new(|bt| {
-        // TODO: Refactor to get rid of `static mut`.
-        #[expect(static_mut_refs, reason = "needs refactoring")]
-        if let Some(f) = unsafe { PANIC_FN.as_ref() } {
-            f.call1(format!("{bt}"));
-        }
+        PANIC_FN.with_borrow(|f| {
+            if let Some(f) = f {
+                f.call1(format!("{bt}"));
+            }
+        });
     }));
 }
 
-/// [`Function`] being called whenever Rust code [`panic`]s.
-static mut PANIC_FN: Option<Function<String>> = None;
+thread_local! {
+    /// [`Function`] being called whenever Rust code [`panic`]s.
+    static PANIC_FN: RefCell<Option<Function<String>>> = RefCell::default();
+}
 
 /// Sets the provided [`Function`] as a callback to be called whenever Rust code
 /// [`panic`]s.
 ///
 /// [`panic`]: panic!
 pub fn set_panic_callback(cb: Function<String>) {
-    unsafe {
-        PANIC_FN = Some(cb);
-    }
+    PANIC_FN.set(Some(cb));
 }
 
 #[cfg(target_os = "android")]
 /// Initializes [`android_logger`] as the default application logger with filter
 /// level set to [`log::LevelFilter::Debug`].
 pub fn init_logger() {
-    // TODO: `android_logger::init_once()` should be called only once.
     android_logger::init_once(
         android_logger::Config::default()
             .with_max_level(log::LevelFilter::Debug),
@@ -107,8 +109,14 @@ pub fn init_logger() {
 /// Initializes [`simple_logger`] as the default application logger with filter
 /// level set to [`log::LevelFilter::Debug`].
 pub fn init_logger() {
-    // TODO: Should be called only once.
-    _ = simple_logger::SimpleLogger::new()
-        .with_level(log::LevelFilter::Debug)
-        .init();
+    thread_local! {
+        /// [`LazyCell`] ensuring that a [`simple_logger`] is initialized only
+        /// once.
+        static INITIALIZED: LazyCell<()> = LazyCell::new(|| {
+            _ = simple_logger::SimpleLogger::new()
+                .with_level(log::LevelFilter::Debug)
+                .init();
+        });
+    }
+    INITIALIZED.with(|i| **i);
 }
diff --git a/src/platform/dart/utils/callback.rs b/src/platform/dart/utils/callback.rs
@@ -220,6 +220,8 @@ extern "C" fn callback_finalizer(_: *mut c_void, cb: *mut c_void) {
 pub mod tests {
     #![expect(clippy::missing_safety_doc, reason = "only for testing")]
 
+    use std::cell::RefCell;
+
     use dart_sys::Dart_Handle;
 
     use crate::api::DartValueArg;
@@ -272,24 +274,29 @@ pub mod tests {
 
     type TestCallbackHandleFunction = extern "C" fn(Dart_Handle);
 
-    static mut TEST_CALLBACK_HANDLE_FUNCTION: Option<
-        TestCallbackHandleFunction,
-    > = None;
+    thread_local! {
+        static TEST_CALLBACK_HANDLE_FUNCTION: RefCell<Option<
+            TestCallbackHandleFunction,
+        >> = RefCell::default();
+    }
 
     #[no_mangle]
     pub unsafe extern "C" fn register__test__test_callback_handle_function(
         f: TestCallbackHandleFunction,
     ) {
-        unsafe {
-            TEST_CALLBACK_HANDLE_FUNCTION = Some(f);
-        }
+        TEST_CALLBACK_HANDLE_FUNCTION.set(Some(f));
     }
 
+    #[expect(clippy::expect_used, reason = "intended behavior")]
     #[no_mangle]
     pub unsafe extern "C" fn test_callback_listener_dart_handle() -> Dart_Handle
     {
         Callback::from_once(move |val: Dart_Handle| {
-            (unsafe { TEST_CALLBACK_HANDLE_FUNCTION.unwrap() })(val);
+            TEST_CALLBACK_HANDLE_FUNCTION.with_borrow(|f| {
+                f.expect("`TEST_CALLBACK_HANDLE_FUNCTION` must be initialized")(
+                    val,
+                );
+            });
         })
         .into_dart()
     }

diff --git a/src/platform/dart/utils/dart_future.rs b/src/platform/dart/utils/dart_future.rs
@@ -160,7 +160,7 @@ impl<O> DartFuture<O> {
     /// transferred to Dart side via `flutter_rust_bridge` bindings.
     #[must_use]
     pub fn into_dart_opaque(self) -> DartOpaque {
-        DartOpaque::new(self.0.cast(), unsafe { DART_HANDLER_PORT.unwrap() })
+        DartOpaque::new(self.0.cast(), DART_HANDLER_PORT.get().unwrap())
     }
 }
 
@@ -208,6 +208,8 @@ where
 pub mod tests {
     #![expect(clippy::missing_safety_doc, reason = "for testing only")]
 
+    use std::cell::RefCell;
+
     use dart_sys::Dart_Handle;
 
     use crate::{
@@ -250,18 +252,20 @@ pub mod tests {
 
     type TestFutureHandleFunction = extern "C" fn(Dart_Handle);
 
-    static mut TEST_FUTURE_HANDLE_FUNCTION: Option<TestFutureHandleFunction> =
-        None;
+    thread_local! {
+        static TEST_FUTURE_HANDLE_FUNCTION: RefCell<
+            Option<TestFutureHandleFunction>
+        > = RefCell::default();
+    }
 
     #[no_mangle]
     pub unsafe extern "C" fn register__test__future_from_dart_handle_fn(
         f: TestFutureHandleFunction,
     ) {
-        unsafe {
-            TEST_FUTURE_HANDLE_FUNCTION = Some(f);
-        }
+        TEST_FUTURE_HANDLE_FUNCTION.set(Some(f));
     }
 
+    #[expect(clippy::expect_used, reason = "intended behavior")]
     #[no_mangle]
     pub unsafe extern "C" fn test__future_from_dart__handle(
         future: Dart_Handle,
@@ -272,7 +276,11 @@ pub mod tests {
                 unsafe { FutureFromDart::execute::<DartHandle>(future.get()) }
                     .await
                     .unwrap();
-            (unsafe { TEST_FUTURE_HANDLE_FUNCTION.unwrap() })(val.get());
+            TEST_FUTURE_HANDLE_FUNCTION.with_borrow(|f| {
+                f.expect("`TEST_FUTURE_HANDLE_FUNCTION` must be initialized")(
+                    val.get(),
+                );
+            });
             Ok(())
         }
         .into_dart_future()

diff --git a/src/platform/dart/utils/string.rs b/src/platform/dart/utils/string.rs
@@ -1,6 +1,7 @@
 //! Helper functionality for passing [`String`]s through FFI boundaries.
 
 use std::{
+    cell::RefCell,
     ffi::{CStr, CString},
     os::raw::c_char,
     ptr,
@@ -11,10 +12,15 @@ use crate::api::propagate_panic;
 /// Pointer to an extern function that frees the provided Dart native string.
 type FreeDartNativeStringFunction = extern "C" fn(ptr::NonNull<c_char>);
 
-/// Stores a pointer to the [`FreeDartNativeStringFunction`] extern function.
-///
-/// Must be initialized by Dart during FFI initialization phase.
-static mut FREE_DART_NATIVE_STRING: Option<FreeDartNativeStringFunction> = None;
+thread_local! {
+    /// Stores a pointer to the [`FreeDartNativeStringFunction`] extern
+    /// function.
+    ///
+    /// Must be initialized by Dart during FFI initialization phase.
+    static FREE_DART_NATIVE_STRING: RefCell<
+        Option<FreeDartNativeStringFunction>
+    > = RefCell::default();
+}
 
 /// Constructs a Rust [`String`] from the provided raw C string.
 ///
@@ -88,20 +94,18 @@ pub unsafe extern "C" fn String_free(s: ptr::NonNull<c_char>) {
 pub unsafe extern "C" fn register_free_dart_native_string(
     f: FreeDartNativeStringFunction,
 ) {
-    unsafe {
-        FREE_DART_NATIVE_STRING = Some(f);
-    }
+    FREE_DART_NATIVE_STRING.set(Some(f));
 }
 
 /// Calls Dart to release memory allocated for the provided native string.
 ///
-/// Should be used when Dart cannot release memory in place, e.g when Rust calls
-/// a Dart function returning a native string.
+/// Should be used when Dart cannot release memory in place, e.g. when Rust
+/// calls a Dart function returning a native string.
 ///
 /// # Safety
 ///
 /// `FREE_DART_NATIVE_STRING` function must be registered and the provided
 /// pointer must be a valid native string.
 pub unsafe fn free_dart_native_string(s: ptr::NonNull<c_char>) {
-    (unsafe { FREE_DART_NATIVE_STRING.unwrap() })(s);
+    FREE_DART_NATIVE_STRING.with_borrow(|f| f.unwrap()(s));
 }