Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add map-like methods to AtomicRef(Mut) which work with new objects rather than references #11

Open
LechintanTudor opened this issue Feb 16, 2021 · 7 comments

Comments

@LechintanTudor
Copy link

Having map methods for reference types which can only work with sub-borrows of the original object is a bit too restrictive in my opinion, and it doesn't allow certain patterns as the one presented bellow. My solution is to add an additional method to all reference types, map_into, which can create entirely new objects with the same lifetime as the original. This change requires adding additional reference types that hold T's, instead of &(mut) T's, which I've called OwnedAtomicRef and OwnedAtomicRefMut.

Here is the problem that adding map_into would solve

let values: (Box<dyn Any>, Box<dyn Any>) = (Box::new(1_u32), Box::new(2_u32));
    let cell = AtomicRefCell::new(values);

    // Create an OwnedAtomicRefMut to hold our new object which borrows the original
    let mut borrowed_values = AtomicRefMut::map_into(cell.borrow_mut(), |values| {
        (
            Box::as_mut(&mut values.0).downcast_mut::<u32>().unwrap(),
            Box::as_mut(&mut values.1).downcast_mut::<u32>().unwrap(),
        )
    });
    
    // Set the values while still holding a lock
    *borrowed_values.0 = 100;
    *borrowed_values.1 = 200;
@LechintanTudor
Copy link
Author

I successfully implemented all the functionality described above. Please let me know if you are interested in a PR.

@bholley
Copy link
Owner

bholley commented Mar 25, 2021

Thanks, and sorry for the lag. So the basic issue here is that you want to use a tuple of borrows, but the tuple is technically not a borrowed type, right?

My intuition is that the duplication across {,Owned}AtomicRef{,Mut} would make the code more complex. So unless we find an elegant way to generalize across the permutations, I'm hesitant to take such a patch absent strong demand from multiple consumers. However, I'll leave this issue open so that others can comment if they run into the same issue.

@LechintanTudor
Copy link
Author

So the basic issue here is that you want to use a tuple of borrows, but the tuple is technically not a borrowed type, right?

Yes that's exactly it.

@zicklag
Copy link

zicklag commented Dec 16, 2022

I have a similar need.

I have a type such as AtomicRef<UntypedStorage>, and there's a method to convert a borrow of &'a UntypedStorage to a TypedStorage<'a, T>, but I couldn't do that in AtomicRef.map because TypedStorage is actually an owned type, with the same lifetime as the &UntypedStorage reference, not a reference to a field of UntypedStorage.

@zicklag
Copy link

zicklag commented Aug 23, 2023

For anybody else who needs it, I was able to accomplish what I needed using the atomicell crate. It has an unsafe method RefMut::into_split() that allows you to get access to the borrow guard and the reference separately, at which point, you must ensure that the reference is never accessed after the borrow guard is dropped, but you can create your own MappedRefMut or whatever you need for your use-case using the guard and the reference.

@Imberflur
Copy link
Contributor

at which point, you must ensure that the reference is never accessed after the borrow guard is dropped, but you can create your own MappedRefMut or whatever you need for your use-case using the guard and the reference.

Note that there is some subtle behavior that you need to be aware of to do this soundly. References passed as function parameters (either directly or included in a type) are treated as if they could be used up until the end of that function even if they aren't used after a certain point (or to put it another way they are expected to not be aliased incorrectly for the entirety of the function).

For an example of how this manifests see #18

To disable this you can wrap the reference/type containing the reference in MaybeDangling from https://docs.rs/maybe-dangling.

For details on why this exists: https://perso.crans.org/vanille/treebor/protectors.html.

@zicklag
Copy link

zicklag commented Aug 23, 2023

Big thanks for pointing that out to me! That's a very easy foot-gun to miss. I'll look into a PR for atomicell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants