Fix a deadlock in a simple channel

Here is an implementation of a simple channel from the book Atomics and Locks:

use std::{
    collections::VecDeque,
    sync::{Condvar, Mutex},
};

pub struct Channel<T> {
    queue: Mutex<VecDeque<T>>,
    items: Condvar,
}

impl<T> Channel<T> {
    pub fn new() -> Self {
        Channel {
            queue: Mutex::new(VecDeque::new()),
            items: Condvar::new(),
        }
    }
    pub fn send(&self, t: T) {
        let mut queue = self.queue.lock().unwrap();
        queue.push_back(t);
        self.items.notify_one();
    }

    pub fn receive(&self) -> Option<T> {
        let mut queue = self.queue.lock().unwrap();
        loop {
            if let Some(t) = queue.pop_front() {
                return Some(t);
            }
            queue = self.items.wait(queue).unwrap();
        }
    }
}

The deadlock occurs if the receive method is called before the send method.
I tried to fix it, but I don't see any good way to do this.
I hope someone can help me with this.

Just to confirm: Are you calling send and receive from two different threads? receive will block (not deadlock) if the queue is empty.

It would probably help for you to post the complete code, with the test, in a playground.

Yes, I calling those two methods from different threads. I use this implementation of a channel for a thread pool

Can you post a test that reproduces the deadlock? I tried but this test passes in the playground.

1 Like

The only thing I can see is that you should drop the guard before notifying.

pub fn send(&self, t: T) {
    let mut queue = self.queue.lock().unwrap();
    queue.push_back(t);
    drop(queue);
    self.items.notify_one();
}

Do you see the deadlock only when using the thread pool? If so, my guess is that there is a bug in the thread pool.

Good point, that's a nice optimization. But it isn't needed for correctness.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.