vorner / arc-swap

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

Can we have `Arc::{get_mut, make_mut}` ? #131

Closed xstater closed 4 months ago

xstater commented 4 months ago

Thank you for providing this great crate.

I was trying to use Arc::{get_mut, make_mut} function. I wrote these code below

let arc_swap = ArcSwap::new(Arc::new(1));

let mut arc = arc_swap.load_full();
// this line will always get None
// because arc_swap hold one reference
// in this time, strong count = 2
let mutable = Arc::get_mut(&mut arc).unwrap();

The Guard can only get the shared reference. I cannot pass it into Arc::get_mut.

So I tried using some unsafe code and unstable feature to implement it

#![feature(get_mut_unchecked)]
let mut arc = arc_swap.load_full();

let strong_count = Arc::strong_count(&arc);

if strong_count == 2 {
    // I cannot proof this unsafe action is safe
    let mutable = unsafe { Arc::get_mut_unchecked(&mut arc) };
}

I thought the unsafe code was safe, because ArcSwap just hold the data and it don't access the data. When strong_count == 2, there is only one which is the arc can mutable access the data. Is it right ?

But if ArcSwap can offer something like get_mut or make_mut. It will be simple. Beacause ArcSwap knows it is the only one reference, maybe can use Arc::get_mut directly.

(please forgive my poor English)

vorner commented 4 months ago

Honestly, I do not think such usage block is safe, on several levels:

On a first glance, having a mutable reference to ArcSwap itself could do this, but it is not true. Guard, despite not sometimes incrementing the reference, does not hold a shared borrow of the originating ArcSwap ‒ the ArcSwap „passes“ the strong count to the existing guard if it is eg. destroyed or replaced under the hood. But that allows code like this:

let mut a = ArcSwap::new(Arc::new(1));
let g = a.load();
assert_eq!(Arc::strong_count(g), 1);
// This would be, as you would have shared ref through g and mutable here
// let r = a.get_mut();
xstater commented 4 months ago

Thank you very much for your detailed reply!

If I understand correctly, there is no safe way to add get_mut or make_mut to ArcSwap.

I am implementing an immutable data structure using Arc::make_mut. It has only one Updater that can always update the internal data, while others can get a "snapshot" of the internal data at any time. I don't want the Updater to call clone on each update. Is there any way to implement this?

vorner commented 4 months ago

If I understand correctly, there is no safe way to add get_mut or make_mut to ArcSwap.

Currently, I don't know of any. I don't have a proof such way doesn't exist, but my feeling is „probably not“.

I don't want the Updater to call clone on each update. Is there any way to implement this?

:thinking: I'm not sure. The „in-place update“ concept kind of goes against the „completely lock-free“ concept, so I'd say not directly. You can't hand out a read snapshot while making an inplace update, so you'd have to wait for it to finish… and that's something ArcSwap can't do (it's the purpose not to do that).

You can either go RwLock<Arc<_>> (then you can do the get_mut while holding the write lock), but you're running the risk of a reader having to wait.

Or you can device the data structure that it clones only a little bit. That is, something like a tree that clones only the path and most of it stays shared. I've always used the im crate for this.

Note that the design of ArcSwap was for scenarios where there's significantly more reading than writing ‒ therefore, it can „afford“ to make writes more expensive if reads are kept cheap.

xstater commented 4 months ago

Wow, Thank you very much for your explanation. I believe using RwLock<Arc<_>> is the better choice. I will now close this issue.