vorner / arc-swap

Support atomic operations on Arc itself
Apache License 2.0
778 stars 31 forks source link

compare_and_swap(None, ...) for ArcSwapOption #61

Closed Ten0 closed 3 years ago

Ten0 commented 3 years ago

It seems we can't use compare_and_swap(None) for ArcSwapOption:

lazy_static::lazy_static! {
    static ref DATA_STORE: ArcSwapOption<SomeStruct> = ArcSwapOption::empty();
}
/// # Panics
/// - If has already been initialized
pub fn init_loading(loading_fn: impl Fn() -> InternalResult<Vec<SomeData>> + Send + Sync + 'static) {
    assert!(DATA_STORE
        .compare_and_swap(
            None,
            Some(Arc::new(SomeStruct {
                data: Vec::new(),
                reload: Arc::new(loading_fn)
            }))
        )
        .is_none())
}
error[E0277]: the trait bound `std::option::Option<_>: AsRaw<KnownCertificatesAndReload>` is not satisfied
  --> file.rs:80:4
   |
80 |         .compare_and_swap(
   |          ^^^^^^^^^^^^^^^^ the trait `AsRaw<KnownCertificatesAndReload>` is not implemented for `std::option::Option<_>

It seems this is not an technical limitation but rather an interface limitation, as AtomicPtr has compare_and_swap for NULL. Is there any way we could achieve this?

vorner commented 3 years ago

Yes, this is not intentional. I'll have a look, hopefully soon.

(As a side note, if this code isn't just a minimized example for demonstration, it seems very similar to what once_cell does)

vorner commented 3 years ago

Quick discovery: It's not implemented for Option<Arc<_>>, but it is for &Option<Arc<_>>, so &None::<Arc<_>> or std::ptr::null() will work.

I'll have to check first if there's a good reason why it isn't implemented for Option directly before adding the implementation.

Ten0 commented 3 years ago

(As a side note, if this code isn't just a minimized example for demonstration, it seems very similar to what once_cell does)

Hmm it is but looks like for my use-case I can indeed use a combination of OnceCell and ArcSwap instead of ArcSwap and Arc. Thanks! :)

vorner commented 3 years ago

So, the reason why it's not for owned is that I get conflicting implementations :-(. I've at least tried to improve the documentation.