Merge pull request #4320 from wmmc88/fix-sync-bounds

Fix sync bounds on LazyLock and OnceLock

embassy-sync/Cargo.toml

@@ -43,3 +43,4 @@ futures-util = { version = "0.3.17", features = [ "channel", "sink" ] }
 # Enable critical-section implementation for std, for tests
 critical-section = { version = "1.1", features = ["std"] }
 static_cell = { version = "2" }
+trybuild = "1.0.105"

embassy-sync/src/lazy_lock.rs

@@ -31,7 +31,12 @@ union Data<T, F> {
     f: ManuallyDrop<F>,
 }
 
-unsafe impl<T, F> Sync for LazyLock<T, F> {}
+unsafe impl<T, F> Sync for LazyLock<T, F>
+where
+    T: Sync,
+    F: Sync,
+{
+}
 
 impl<T, F: FnOnce() -> T> LazyLock<T, F> {
     /// Create a new uninitialized `StaticLock`.

embassy-sync/src/once_lock.rs

@@ -42,7 +42,7 @@ pub struct OnceLock<T> {
     data: Cell<MaybeUninit<T>>,
 }
 
-unsafe impl<T> Sync for OnceLock<T> {}
+unsafe impl<T> Sync for OnceLock<T> where T: Sync {}
 
 impl<T> OnceLock<T> {
     /// Create a new uninitialized `OnceLock`.

embassy-sync/tests/ui.rs

@@ -0,0 +1,13 @@
+#[cfg(not(miri))]
+#[test]
+fn ui() {
+    let t = trybuild::TestCases::new();
+
+    // These test cases should fail to compile since OnceLock and LazyLock should not unconditionally implement sync
+    // for all types. These tests are regression tests against the following issues:
+    // * https://github.com/embassy-rs/embassy/issues/4307
+    // * https://github.com/embassy-rs/embassy/issues/3904
+    t.compile_fail("tests/ui/sync_impl/lazy_lock_function.rs");
+    t.compile_fail("tests/ui/sync_impl/lazy_lock_type.rs");
+    t.compile_fail("tests/ui/sync_impl/once_lock.rs");
+}

embassy-sync/tests/ui/sync_impl/lazy_lock_function.rs

@@ -0,0 +1,11 @@
+use embassy_sync::lazy_lock::LazyLock;
+
+fn main() {
+    let x = 128u8;
+    let x_ptr: *const u8 = core::ptr::addr_of!(x);
+    let closure_capturing_non_sync_variable = || unsafe { core::ptr::read(x_ptr) };
+
+    check_sync(LazyLock::new(closure_capturing_non_sync_variable));
+}
+
+fn check_sync<T: Sync>(_lazy_lock: T) {}

embassy-sync/tests/ui/sync_impl/lazy_lock_function.stderr

@@ -0,0 +1,24 @@
+error[E0277]: `*const u8` cannot be shared between threads safely
+  --> tests/ui/sync_impl/lazy_lock_function.rs:8:16
+   |
+6  |     let closure_capturing_non_sync_variable = || unsafe { core::ptr::read(x_ptr) };
+   |                                               -- within this `{closure@$DIR/tests/ui/sync_impl/lazy_lock_function.rs:6:47: 6:49}`
+7  |
+8  |     check_sync(LazyLock::new(closure_capturing_non_sync_variable));
+   |     ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*const u8` cannot be shared between threads safely
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: within `{closure@$DIR/tests/ui/sync_impl/lazy_lock_function.rs:6:47: 6:49}`, the trait `Sync` is not implemented for `*const u8`
+   = note: required because it appears within the type `&*const u8`
+note: required because it's used within this closure
+  --> tests/ui/sync_impl/lazy_lock_function.rs:6:47
+   |
+6  |     let closure_capturing_non_sync_variable = || unsafe { core::ptr::read(x_ptr) };
+   |                                               ^^
+   = note: required for `embassy_sync::lazy_lock::LazyLock<u8, {closure@$DIR/tests/ui/sync_impl/lazy_lock_function.rs:6:47: 6:49}>` to implement `Sync`
+note: required by a bound in `check_sync`
+  --> tests/ui/sync_impl/lazy_lock_function.rs:11:18
+   |
+11 | fn check_sync<T: Sync>(_lazy_lock: T) {}
+   |                  ^^^^ required by this bound in `check_sync`

embassy-sync/tests/ui/sync_impl/lazy_lock_type.rs

@@ -0,0 +1,6 @@
+use embassy_sync::lazy_lock::LazyLock;
+
+// *mut u8 is not Sync, so LazyLock should not implement Sync for this type. This should fail to compile.
+static GLOBAL: LazyLock<*mut u8> = LazyLock::new(|| core::ptr::null_mut());
+
+fn main() {}

embassy-sync/tests/ui/sync_impl/lazy_lock_type.stderr

@@ -0,0 +1,9 @@
+error[E0277]: `*mut u8` cannot be shared between threads safely
+ --> tests/ui/sync_impl/lazy_lock_type.rs:4:16
+  |
+4 | static GLOBAL: LazyLock<*mut u8> = LazyLock::new(|| core::ptr::null_mut());
+  |                ^^^^^^^^^^^^^^^^^ `*mut u8` cannot be shared between threads safely
+  |
+  = help: the trait `Sync` is not implemented for `*mut u8`
+  = note: required for `embassy_sync::lazy_lock::LazyLock<*mut u8>` to implement `Sync`
+  = note: shared static variables must have a type that implements `Sync`

embassy-sync/tests/ui/sync_impl/once_lock.rs

@@ -0,0 +1,6 @@
+use embassy_sync::once_lock::OnceLock;
+
+// *mut u8 is not Sync, so OnceLock should not implement Sync for this type. This should fail to compile.
+static GLOBAL: OnceLock<*mut u8> = OnceLock::new();
+
+fn main() {}

embassy-sync/tests/ui/sync_impl/once_lock.stderr

@@ -0,0 +1,9 @@
+error[E0277]: `*mut u8` cannot be shared between threads safely
+ --> tests/ui/sync_impl/once_lock.rs:4:16
+  |
+4 | static GLOBAL: OnceLock<*mut u8> = OnceLock::new();
+  |                ^^^^^^^^^^^^^^^^^ `*mut u8` cannot be shared between threads safely
+  |
+  = help: the trait `Sync` is not implemented for `*mut u8`
+  = note: required for `embassy_sync::once_lock::OnceLock<*mut u8>` to implement `Sync`
+  = note: shared static variables must have a type that implements `Sync`