bholley/atomic_refcell

Threadsafe RefCell for Rust

atomic_refcell's Issues

Improve mapping features

There's an already open request about an owned_map feature that return a non-borrowed object.
Please consider improving the API with other construct as well:

• map : &T -> &(mut) U
• filter_map : &T -> Option<&(mut) U>
• try_map : &T -> Result<&(mut) U, E>
• and their "owning" version as described in #11

I felt like this is a bit broader request as the #11, thus I've created a new issue.

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

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;

Bounds for `AtomicRefCell<T>: Send` are unnecessarily restrictive

Currently AtomicRefCell<T>: Send requires T: Send + Sync. I believe this can be relaxed to T: Send.

Also, see: rust-lang/rust#42934

Remove Sync constraints ?

I believe that AtomicRefCell can guarantee that there is only one mutable reference at some time like a mutex, so the Sync constraint should be unnecessary.

https://docs.rs/atomic_refcell/0.1.7/atomic_refcell/struct.AtomicRefCell.html#impl-Send
https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#impl-Send

AtomicRefMut is not Sync nor Send in async block

The switch from 0.1.9 to 0.1.10 increases the requirements for AtomicRef and AtomicRefMut to be Sync or Send.
This makes async blocks fail to compile with a very weird higher-ranked lifetime error

Here an example:

use std::future::Future;

use atomic_refcell::AtomicRefCell;

fn spawn<Fut>(_future: Fut)
where
    Fut: Future<Output = ()> + Send,
{
}

async fn other() {}

#[test]
fn test_spawn() {
    let arc: Box<dyn Fn() -> () + Send + Sync> = Box::new(|| println!("hello"));
    let a = AtomicRefCell::new(arc);
    spawn(async move {
        let x = a.borrow_mut();
        other().await;
        println!("hello {:?}", x());
    });
}

Gives

error: higher-ranked lifetime error
  --> tests/async.rs:17:5
   |
17 | /     spawn(async move {
18 | |         let x = a.borrow_mut();
19 | |         other().await;
20 | |         println!("hello {:?}", x());
21 | |     });
   | |______^
   |
   = note: could not prove `[async block@tests/async.rs:17:11: 21:6]: Send`

error: could not compile `atomic_refcell` (test "async") due to previous error

(note: here it only shows Send because spawn only requires Send and not Sync)

The change was made here:
0690641#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R468

Note, how the documentation changes from

to

#[cfg(feature = "debug_atomic_refcell")] ?

In the original RefCell implementation there is a cfg feature called "debug_refcell", which in case of a panic allows to see where a RefCell was first borrowed:

pub struct RefCell<T: ?Sized> {
    borrow: Cell<BorrowFlag>,
    // Stores the location of the earliest currently active borrow.
    // This gets updated whenever we go from having zero borrows
    // to having a single borrow. When a borrow occurs, this gets included
    // in the generated `BorrowError/`BorrowMutError`
    #[cfg(feature = "debug_refcell")]
    borrowed_at: Cell<Option<&'static crate::panic::Location<'static>>>,
    value: UnsafeCell<T>,
}

I read that you stripped RefCell of everything that conflicted multi-thread environment. Is it because of this that the debug feature was removed? Is it hard to reimplement it back? Pushing borrow checking into runtime leaves us with less info to investigate things. This leads projects like accountable-refcell to store the whole stack trace per RefCell. Well, that much of an overhead is probably overkill, but to have the minimal "borrowed_at" info is an essential thing to have for debug builds, don't you think?🤔

Perfomance?

Could you provide perfomance data?
Comparing to Mutex, RwLock...

License

Hello.

Would it be possible to add a license file to this repo, please?

AtomicWeakRef

Would it be possible to add a weak reference type to this library that doesn't include a lifetime parameter?
I've been trying to create something similar to "Box" but with thread-safe weak pointers;
or "Arc" but with the ability to into_inner and get the original value out.

Your library almost satisfies my requirements, but the lifetime requirement on the AtomicRef was problematic.

For reference:
https://www.reddit.com/r/rust/comments/1aq8zzf/help_implementing_an_atomic_reference_box_type/

Note that in my partial-implementation, the into_inner function can block until the weak references aren't actively borrowing. Not sure how this behavior would be accomplished in your library.

Create a Security Policy

A Security Policy is a GitHub standard document (SECURITY.md) that can be seen in the "Security Tab" to instruct users about how to report vulnerability in the safest and most efficient way possible.

It is a Scorecard Recommendation (being one check of medium priority) and a Github Recommendation.

Together with this issue I'll submit one suggestion of Security Policy, feel free to edit it directly or ask me for editions until it is in compliance with how atomic_refcell would best handle vulnerability reports.