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

Deadlock when working within async threads #79

Closed
linxiulei opened this issue Apr 14, 2020 · 9 comments
Closed

Deadlock when working within async threads #79

linxiulei opened this issue Apr 14, 2020 · 9 comments
Labels

Comments

@linxiulei
Copy link

linxiulei commented Apr 14, 2020

Following code hang forever

use dashmap::DashMap;
use futures::future::join_all;
use std::time::Duration;
use tokio;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let tasks: DashMap<i32, i32> = DashMap::new();
    let t = &tasks;
    let mut async_events = vec![];
    for i in 0..30 {
        tasks.insert(i, i);
        async_events.push(async move {
            let j = i;
            if let Some(_) = t.get(&j) {
                tokio::time::delay_for(Duration::from_secs(1)).await;
            }

            if let Some(_) = t.get_mut(&j) {
                tokio::time::delay_for(Duration::from_secs(1)).await;
            }
        });
    }
    join_all(async_events).await;
    println!("Hello, world!");
    Ok(())
}

To make it simple to explain, say there are 2 coroutines and 1 thread underlying.

  1. coroutine A hold the read lock of shard a1 and yield itself to coroutine B
  2. coroutine B try to get the write lock of shard a1, but fails. So it spins on trying to get it but never yield itself

The above might be the cause of the hanging issue. However, I don't think it's going to be fixed by Dashmap(no way to yield or notify tokio to reschedule), at least it's not easy.

If we're not going to fix it should we warn users not to use it with tokio in similar usecases?

@xacrimon
Copy link
Owner

This is a known issue. It stems from the fact that you are keeping a Ref guard bound to _ across an await point which may deadlock. The v4 release does fix this issue and will be coming out in May.

@xacrimon xacrimon assigned xacrimon and unassigned xacrimon Apr 14, 2020
@xacrimon xacrimon added bug Something isn't working v3 labels Apr 14, 2020
@xacrimon xacrimon mentioned this issue Apr 19, 2020
Closed
12 tasks
@xacrimon xacrimon linked a pull request Apr 19, 2020 that will close this issue
Closed
12 tasks
@xacrimon xacrimon reopened this Apr 19, 2020
@xacrimon xacrimon removed the bug Something isn't working label Apr 20, 2020
@xacrimon
Copy link
Owner

I've released the mostly feature complete version 4.0.0-rc4 now. Can update to that and then close this issue?

@d4h0 d4h0 mentioned this issue Jul 27, 2020
@comicfans
Copy link

Hello, I tried to run this example code with rust 1.51 stable toolchain 2018 edition, dashmap v4.0.2, with tokio 0.2.25/0.3.7/1.5.0, all of them run for ever ? (I'm running on linux x86-64)

@cetra3
Copy link

cetra3 commented May 26, 2021

I can confirm this is still an issue when holding write locks over await points, it looks like the PR for the original v4 was closed: #81 so this issue is still open.

It'd be great if there was an alternative that used rwlocks from tokio under the hood.

@JakkuSakura
Copy link

JakkuSakura commented Jun 30, 2021

Maybe async_rwlock is a good idea. async_rwlock is runtime agnostic. Acquiring the lock may block the whole thread, which is something I worry about.

@xacrimon
Copy link
Owner

Yeah, that makes perfect sense. Generally this isn't a problem as tokio advises using regular locks anyway for protected data even in async code but the await point problem needs to be looked into. When I have time I am looking to add a variant that uses async locks but I cannot provide a timeframe for this as I don't get paid for this work.

@gitmalong
Copy link

gitmalong commented Jul 3, 2022

Any update regarding that?

@notgull
Copy link

notgull commented Jul 4, 2022

I think "don't hold a lock across a .await" should be documented.

@gitmalong
Copy link

#150

In general though, I suggest not keeping refs across await points and sync locks are also fine to use in async as per tokio docs

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

Successfully merging a pull request may close this issue.

7 participants