Make DocHande !Sync #57
Conversation
| @@ -30,6 +32,7 @@ pub struct DocHandle { | |||
| /// that doesn't require a mutabale reference to the handle. | |||
| /// Note: the mutex is not shared between clones of the same handle. | |||
| last_heads: Mutex<Vec<ChangeHash>>, | |||
| _not_sync: PhantomData<Cell<&'static ()>>, | |||
There was a problem hiding this comment.
| async fn run_bakery_algorithm(doc_handle: &DocHandle, customer_id: &String) { | ||
| let (our_number, closing) = doc_handle.with_doc_mut(|doc| { | ||
| async fn run_bakery_algorithm(doc_handle: &Mutex<DocHandle>, customer_id: &String) { | ||
| let (our_number, closing) = doc_handle.lock().await.with_doc_mut(|doc| { |
There was a problem hiding this comment.
I think the locking behaviour here and elsewhere is probably wrong, but I'm not sure what to do...
@gterzian if you could advise?
Enforce at the type level that DocHandle is not to be shared across threads.
teohhanhui
force-pushed
the
doc-handle-not-sync
branch
from
f929ea8
to
8739b3a
Compare
|
@teohhanhui Thanks for the PR! couple of questions after a quick look:
I'm guessing the mutex is necessary because tokio wants tasks to by sync? In that case it means we unfortunately cannot specify |
The mutex is necessary because of tokio-rs/axum#1463 |
Ok I see, tokio only requires futures to be I think for now I would rather keep the doc handle as it is, and have a kind of loose convention that if relying on A good example would be https://docs.rs/tokio/latest/tokio/sync/watch/struct.Receiver.html, which also is |
Actually it's documented to be fine: https://docs.rs/tokio/latest/tokio/sync/watch/index.html#thread-safety
Uhh, I see... That's fair, i.e. using it concurrently is technically correct but what you probably want to do is to clone it. |
Exactly |
Enforce at the type level that DocHandle is not to be shared across threads.
/cc @gterzian: bowtieworks/automerge_orm#3 (comment)