jorendorff / atomicbox Goto Github PK

What do you think about extending the AtomicOptionBox API with a function that only replaces the current content of an AtomicOptionBox if it's currently None?

Possible signature (subject to bikeshedding):

pub fn store_if_none(&self, other: Box<T>, order: Ordering) -> Result<(),Box<T>>

The Err content of the returned Result in case of failure is the other argument.

The implementation can use AtomicPtr::compare_exchange.

If this makes sense to you, I can prepare a PR with an implementation.

Motivation

My current use-case for AtomicOptionBox is as a cross-thread communication mechanism in a context where queuing is undesirable.
AtomicOptionBox::store works well for message types in which the latest takes precedence, so the sender can overwrite the older message regardless of whether the receiver got it or not.
The proposed AtomicOptionBox::store_if_none will work in an inverse situation, when the earliest message takes precedence.
The same can be achieved with std::sync::mpsc::sync_channel(bound=1) and try_send, but AtomicOptionBox, arguably, better expresses intent, and in my use-case would avoid adding a different communication mechanism.

Consider adding something like this:

impl<T> AtomicOptionBox<T> {
    pub fn is_some(&self, ordering: Ordering) -> bool;
}

It would be helpful for checking whether the box has a value without taking it. A simple use case is just for implementing Debug without exposing the box itself. Also for implementing channels and other concurrent structures.

Currently, AtomicOptionBox::take only allows AcqRel and SeqCst, just like AtomicOptionBox::swap. But AtomicOptionBox::take is always putting a None into the box, which doesn't reference any other data. It still needs to be Acquire (so that it can safely drop the possibly-not-None value that it takes out), but I don't think it needs to be Release as well.

AtomicBox has blanket Sync+Send impls, which allows non-Send items to be sent to other threads.

I believe it's safe for AtomicBox<T> to be Sync even if T is only Send, (same as Mutex<T>) but it's definitely not safe for AtomicBox<T> to be Sync+Send if T is neither.

Sample: Atomic Box allows Rc to be sent to other thread (tested with atomicbox = "0.3.0")

use std::rc::Rc;
use atomicbox::AtomicBox;

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 || {
        let value = *abox.into_inner();
        println!("Other thread: {:?}", value);
    });
    handle.join().unwrap();
    println!("Main Thread: {:?}", rc_main);
}

I was thinking of using AtomicOptionBox in a static variable, which can only be initialized using a const fn. Is the following worth adding to the library?

impl<T> AtomicOptionBox<T> {
    ...
    pub const fn new_none() -> AtomicOptionBox<T> {
        AtomicOptionBox {
            ptr: AtomicPtr::new(null_mut()),
        }
    }