Use futures::lock::Mutex for sharing state between requests

[allevo]

Hi!
I'm working with futures (i.e my handlers are async)

Ofter I need to protect my Service around Arc<Mutex<_>> where Arc and Mutex came from std. But when you are working with futures, it's better to use futures::lock::Mutex in order to avoid dead lock (see for example https://users.rust-lang.org/t/std-mutex-vs-futures-mutex-vs-futures-lock-mutex-for-async/41710/2)
So,

use futures::lock::Mutex;
use std::sync::{Arc};

#[derive(Clone, StateData)]
pub struct ChatServiceWrapper {
    pub inner: Arc<Mutex<ChatService>>,
}

    let chat_service = ChatServiceWrapper {
        inner: Arc::new(Mutex::new(ChatService::new())),
    };
    let pipeline = new_pipeline()
        .add(StateMiddleware::new(chat_service)) // <--- this goes to error
        .build()

the error is

the type `std::cell::UnsafeCell<chat_service::ChatService>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary

`std::cell::UnsafeCell<chat_service::ChatService>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary

help: within `rest_api::ChatServiceWrapper`, the trait `std::panic::RefUnwindSafe` is not implemented for `std::cell::UnsafeCell<chat_service::ChatService>`

How can I fix this error??

Thanks a lot

[msrd0]

Hm ... this is not so easy. What the error says is basically that the compiler needs to prove that, if any of your handler methods panic, the gotham server can "unwind" that panic and continue running, without leaving the server in any sort of invalid state.

std::sync::Mutex also uses UnsafeCell internally but manually implements the RefUnwindSafe marker trait, indicating to the compiler that it can be safely used after a handler paniced. I'm not sure if futures_util::lock::Mutex has an implementation that doesn't guarantee its references to be unwind safe, or if they simply forgot to implement that marker trait. The implementations at least differ in that std implements poisioning, whereas futures does not seem to get poisioned. In the first case, you'll need to look for a different mutex. In the second case, I'd recommend you wrap it in a custom struct, like so:

pub struct Mutex<T>(futures_util::lock::Mutex<T>);

// ensure that this is indeed safe!!!!!!!!!
impl<T: ?Sized> RefUnwindSafe for Mutex {}

// these allow calling all the mutex functions
impl<T: ?Sized> Deref for Mutex<T> { ... }
impl<T: ?Sized> DerefMut for Mutex<T> { ... }

[allevo]

Apparently this is not safe https://stackoverflow.com/questions/69546701/why-does-futureslockmutex-not-implement-refunwindsafe

I got your point but in my case, I'm working only async handler.
So it not make any sense to use catch_unwind . Truly, there's a similar method for futures here https://docs.rs/futures/0.3.17/futures/future/trait.FutureExt.html#method.catch_unwind

Is there any chance to disable catch_unwind capability or substitute with the FutureExt method?
How could it be difficult to implement?

I can study a bit a try to implement the feature, if you want.

[msrd0]

Gotham already uses FutureExt::catch_unwind for handlers. Also, all handlers run asynchronously, no matter if you call .to or .to_async - the first one essentially wraps everything in an async {} block.

There is no way to disable catching panics and I have no plan on adding support for that.

I recommend you use some mutex implementation that supports poisioning while being future aware. Unfortunately I don't know any such implementations, but I assume someone has encountered that problem before.

[allevo]

But std::sync::Mutex are not safe to use with futures/async/await for dead locks. I have to use futures::lock::Mutex in order to avoid them.
But futures::lock::Mutex doesn't implement RefUnwindSafe because it is not safe, so it doens't compile.

Things go round and round, is there any chance to resolve this problem in somehow?

Thanks for the support

[msrd0]

Hm I can't find a crate that implements a mutex that is future aware and supports poisioning ...

I guess you could always use spawn_blocking together with a std mutex.

[tanriol]

But std::sync::Mutex are not safe to use with futures/async/await for dead locks. I have to use futures::lock::Mutex in order to avoid them.

It's safe to use std::sync::Mutex with futures/async. Formally speaking, deadlocking is safe (though it's not something you want usually).

You don't risk deadlocking (any more than usual) as long as you don't .await while holding a mutex. Making this mistake is a bit hard because a future doing that won't be Send, and spawning futures usually requires Send.

Also note that keeping a Mutex for a long time, especially while waiting for an external event, is a bad idea with any mutexes, whether futures aware or not.

[msrd0]

@allevo Has your question been answered?

[allevo]

Yes, but unfortunally there's no way to do that well:

• I cannot store a futures::lock::Mutex inside the StateData because it doens't implement RefUnwindSafe (and it is not safe)
• I cannot store std::sync::Mutex<MyService> inside the StateData because MyService cannot have async method because the MutexGuard is not sharable between Thread. (this is required because the mutexguard is dropped after the await on the method invocation).

So, the initial question is answered but I don't find a good solution ...

[tanriol]

In this situation I'd try to make MyService methods take &self so that a mutex is not needed at all, and handle the required locking internally.

[allevo]

Yes but that way moves the problem inside: if the method locks the mutex and after makes another await, the problem remains.