Make AtomicBox<T> non-Sync when T is non-Send.

Fixes a soundness bug reported by @geeklint. Closes #5.

This breaks API compatibility only for programs that are doing something very
unsafe, but a version bump seems appropriate anyway.

Cargo.toml

@@ -2,11 +2,14 @@
 name = "atomicbox"
 edition = "2021"
 description = "Safe atomic pointers to boxed data."
-version = "0.3.0"
+version = "0.4.0"
 authors = ["Jason Orendorff <jason.orendorff@gmail.com>"]
 categories = ["concurrency", "no-std"]
 license = "MIT/Apache-2.0"
 readme = "README.md"
 repository = "https://github.com/jorendorff/atomicbox"
 
 [dependencies]
+
+[dev-dependencies]
+compiletest_rs = "0.7.1"

src/atomic_box.rs

@@ -1,5 +1,6 @@
 use alloc::boxed::Box;
 use core::fmt::{self, Debug, Formatter};
+use core::marker::PhantomData;
 use core::mem::forget;
 use core::ptr;
 use core::sync::atomic::{AtomicPtr, Ordering};
@@ -8,8 +9,19 @@ use core::sync::atomic::{AtomicPtr, Ordering};
 /// threads.
 pub struct AtomicBox<T> {
     ptr: AtomicPtr<T>,
+
+    /// This effectively makes `AtomicBox<T>` non-`Send` and non-`Sync` if `T`
+    /// is non-`Send`.
+    phantom: PhantomData<Box<T>>,
 }
 
+/// Mark `AtomicBox<T>` as safe to share across threads.
+///
+/// This is safe because shared access to an `AtomicBox<T>` does not provide
+/// shared access to any `T` value. However, it does provide the ability to get
+/// a `Box<T>` from another thread, so `T: Send` is required.
+unsafe impl<T> Sync for AtomicBox<T> where T: Send {}
+
 impl<T> AtomicBox<T> {
     /// Creates a new `AtomicBox` with the given value.
     ///
@@ -22,6 +34,7 @@ impl<T> AtomicBox<T> {
     pub fn new(value: Box<T>) -> AtomicBox<T> {
         AtomicBox {
             ptr: AtomicPtr::new(Box::into_raw(value)),
+            phantom: PhantomData,
         }
     }
 

src/atomic_option_box.rs

@@ -1,5 +1,6 @@
 use alloc::boxed::Box;
 use core::fmt::{self, Debug, Formatter};
+use core::marker::PhantomData;
 use core::mem::forget;
 use core::ptr::null_mut;
 use core::sync::atomic::{AtomicPtr, Ordering};
@@ -10,8 +11,19 @@ pub struct AtomicOptionBox<T> {
     /// Pointer to a `T` value in the heap, representing `Some(t)`;
     /// or a null pointer for `None`.
     ptr: AtomicPtr<T>,
+
+    /// This effectively makes `AtomicOptionBox<T>` non-`Send` and non-`Sync`
+    /// if `T` is non-`Send`.
+    phantom: PhantomData<Box<T>>,
 }
 
+/// Mark `AtomicOptionBox<T>` as safe to share across threads.
+///
+/// This is safe because shared access to an `AtomicOptionBox<T>` does not
+/// provide shared access to any `T` value. However, it does provide the
+/// ability to get a `Box<T>` from another thread, so `T: Send` is required.
+unsafe impl<T> Sync for AtomicOptionBox<T> where T: Send {}
+
 fn into_ptr<T>(value: Option<Box<T>>) -> *mut T {
     match value {
         Some(box_value) => Box::into_raw(box_value),
@@ -39,6 +51,7 @@ impl<T> AtomicOptionBox<T> {
     pub fn new(value: Option<Box<T>>) -> AtomicOptionBox<T> {
         AtomicOptionBox {
             ptr: AtomicPtr::new(into_ptr(value)),
+            phantom: PhantomData,
         }
     }
 
@@ -56,6 +69,7 @@ impl<T> AtomicOptionBox<T> {
     pub const fn none() -> Self {
         Self {
             ptr: AtomicPtr::new(null_mut()),
+            phantom: PhantomData,
         }
     }
 

tests/compile-fail/send_rc.rs

@@ -0,0 +1,18 @@
+extern crate atomicbox;
+
+use atomicbox::AtomicBox;
+use std::rc::Rc;
+
+fn main() {
+    let rc_main = Rc::new("Hello, world!");
+    let rc_other = Rc::clone(&rc_main);
+
+    let abox = AtomicBox::new(Box::new(rc_other));
+    let handle = std::thread::spawn(move || {
+        //~^ ERROR `Rc<&str>` cannot be sent between threads safely
+        let value = *abox.into_inner();
+        println!("Other thread: {:?}", value);
+    });
+    handle.join().unwrap();
+    println!("Main Thread: {:?}", rc_main);
+}

tests/compile_tests.rs

@@ -0,0 +1,24 @@
+#![allow(clippy::field_reassign_with_default)]
+
+// Copied from <https://lib.rs/crates/compiletest_rs>.
+
+extern crate compiletest_rs as compiletest;
+
+use std::path::PathBuf;
+
+fn run_mode(mode: &'static str) {
+    let mut config = compiletest::Config::default();
+
+    config.mode = mode.parse().expect("Invalid mode");
+    config.src_base = PathBuf::from(format!("tests/{}", mode));
+    config.target_rustcflags = Some("-L target/debug".to_string());
+    config.link_deps(); // Populate config.target_rustcflags with dependencies on the path
+    config.clean_rmeta(); // If your tests import the parent crate, this helps with E0464
+
+    compiletest::run_tests(&config);
+}
+
+#[test]
+fn compile_test() {
+    run_mode("compile-fail");
+}