with_upgraded methods on upgradable read guards

[Jules-Bertholet]

Allow temporarily upgrading an upgradable read guard to a write guard given only a mutable reference to the guard.

RwLockUpgradableReadLockArc::with_upgraded_blocking, along with the other missing blocking arc rwlock methods, is left to future work.

Investigating whether it's possible to have sound async equivalents of these methods is also left to future work.

[notgull]

Not sure about this API. Feels like you can do the same thing with:

let mut guard = Some(...);

{
    let mut upgraded = guard.take().unwrap().upgrade().await;
    f(&mut upgraded);
    guard = Some(upgraded.downgrade());
}

[Jules-Bertholet]

Yes, and in fact that's the workaround I use now. However, this PR's API has 2 advantages:

• No Option (no unwrap()s, potential minor space savings)
• Properly handles the case where f panics (if the panic is caught, the guard remains accessible in the upgradable state)

[notgull]

• No Option (no unwrap()s, potential minor space savings)

The guards are niched (at least I think they are) so there shouldn't be any space wasted. The movement here amount to around four small memcpy's that the compiler probably optimizes away anyways.

• Properly handles the case where f panics (if the panic is caught, the guard remains accessible in the upgradable state)

This is still possible:

let mut guard = Some(...);

{
    let upgraded = guard.take().unwrap().upgrade().await;

    struct RestoreOnDrop<'a, T> {
        value: T,
        slot: &'a mut Option<T>
    }

    impl<'a, T> Drop for RestoreOnDrop<'a, T> {
        fn drop() {
            *self.slot = Some(value.downgrade());
    }

    let mut upgraded = RestoreOnDrop {
        value: upgraded,
        slot: &mut guard,
    };
    f(&mut upgraded.value);
}

Yes, this is a little bit more code. However, it's a truly cursed use case if you still need your guard held on your RwLock to be preserved after a panic. It seems like a niche use case for the addition of a public API. We should be careful with this kind of public API, as it's exposed in both smol and async-std. Especially since mixing futures and callbacks is very unidiomatic.

[Jules-Bertholet]

To explain my use-case:

I have a resource (represented by a struct), that's managed by a long-running async task. The task regularly reads from the resource, and occasionally writes; the write operations are short and don't cross awaits. Other tasks will read from time to time, but never write. What I do is, I have the task own a copy of an RwLockUpgradableReadLockArc<Resource>, and pass it by &mut to various methods to perform different operations. Those methods use with_upgraded when they need to write. Except that currently, RwLockUpgradableReadLockArc has no with_upgraded, so all the methods take a newtype wrapper around Option<RwLockUpgradableReadLockArc<_>> instead. Having this API in async-lock would save me the trouble of maintaining the wrapper type. It's only a minor inconvenience, to be sure; but I suppose I can't be the only person to have ever had a similar use-case, and I see no harm in centralizing the solution.

[zeenix]

To explain my use-case:

Thanks for explaining in great detail but as @notgull pointed out, this seems like a very niche use case.

What I do is, I have the task own a copy of an RwLockUpgradableReadLockArc<Resource>, and pass it by &mut to various methods to perform different operations. Those methods use with_upgraded when they need to write.

Why not pass an Arc<RwLock> instead? Even though methods don't need mutability, you can still have the methods take an &mut Arc<RwLock> if exterior mutability is needed there for some reason as well.

[Jules-Bertholet]

Why not pass an Arc<RwLock> instead?

So I can Deref the upgradable lock directly. Having to take the read lock in every method would just be pointless sytactic noise.

[notgull]

I'm going to say that we shouldn't add this. It's a pretty niche use case that doesn't fit well with the rest of the API, and it can be worked around for these niche use cases. I'd say that the cost of adding this API and maintaining it for the foreseeable future outweighs the potential utility it would provide.

Thanks anyways!