feature request: try_rcu / some return value to the caller of rcu

[rbtcollins]

Sometimes the closure passed to rcu() needs to be observable to the caller.

e.g.

i = **as.load()
as.rcu(|i| if *i > 10 {  i }  else {*i + 1});
if i == **as.load() {
   Err("hit the limit")
} else {Ok(())}

But this is clearly racy as we are re-reading.

I would love to be able to do:

as.try_rcu(|i| if *i > 10 {     Err("hit the limit")  }  else { Ok(*i + 1)});

or some similar construct, where the computation within the closure is able to signal to the caller.

Would you like a patch?

[vorner]

Hello

To be honest, I kind of don't understand what you're trying to say/accomplish. Maybe you could sketch the signature of that try_rcu?

Does it help that the rcu returns the previous value or that the closure passed in is FnMut, therefore you can „export“ any notes outside, like

bool changed = false;
as.rcu(|i| if (*i > 10) { i } else { changed = true; *i + 1 });
if changed { ... }

Anyway, sometimes I feel like the rcu itself is already a bit too complicated bit of API, considering the lowlevelness of the whole thing… therefore I don't really enjoy the idea of adding something even more complex. Isn't it enough that anything like that can be quite easily built with compare_and_swap (with inspiration by looking at the source code of rcu)?

[ilammy]

I've been using the following extension trait for a while to implement try_rcu.

pub trait ArcSwapExt<T> {
    /// [`ArcSwap::rcu`](arc_swap::ArcSwap::rcu), but with Result that short-circuits on error.
    fn try_rcu<R, F, E>(&self, f: F) -> Result<T, E>
    where
        F: FnMut(&T) -> Result<R, E>,
        R: Into<T>;
}

impl<T, S> ArcSwapExt<T> for arc_swap::ArcSwapAny<T, S>
where
    T: arc_swap::RefCnt,
    S: arc_swap::strategy::CaS<T>,
{
    fn try_rcu<R, F, E>(&self, mut f: F) -> Result<T, E>
    where
        F: FnMut(&T) -> Result<R, E>,
        R: Into<T>,
    {
        fn ptr_eq<Base, A, B>(a: A, b: B) -> bool
        where
            A: arc_swap::AsRaw<Base>,
            B: arc_swap::AsRaw<Base>,
        {
            let a = a.as_raw();
            let b = b.as_raw();
            std::ptr::eq(a, b)
        }

        let mut cur = self.load();
        loop {
            let new = f(&cur)?.into();
            let prev = self.compare_and_swap(&*cur, new);
            let swapped = ptr_eq(&*cur, &*prev);
            if swapped {
                return Ok(arc_swap::Guard::into_inner(prev));
            } else {
                cur = prev;
            }
        }
    }
}

ArcSwap's API has been stable for years, so was this code. Would be nice to move it here, but it's not a big deal to keep it around like that.

My use case is fallible updates. For example, adding an item to a persistent collection wrapped in ArcSwap or failing and not changing anything if that's a duplicate.

[vorner]

Hmm, now I see the purpose.

My little worry here is, if it works for Result<.., ..>, should it also work for Option and for futures, etc. Unfortunately, Try is not stable yet :-(.

So I have to wonder if:

• Better let these things live outside.
• Do just this one for Result
• Depend on a nightly feature (I don't like this one here)
• Introduce our own Try for Result and Option, and ControlFlow (under a feature gate to allow compiling on old rusts).

None of them looks like the ideal solution, but I'm open to your views on what to do here.

[rbtcollins]

I don't have a strong opinion on the details, other than it would just be nice to have the feature.

Result is more general than Option - Result<T, ()> can model an Option, but not vice versa, so my suggestion would be to either implement just Result until Try is stable, and then revisit after stabilisation.

[vorner]

OK, that's fair. That being said, I'm not 100% sure extanding that try_rcu for the future Try would be a semver-compatible change at that point. But we can leave it until then.

Would you send a PR?

(And sorry that my answer took so long, too many other things to attend to…)