notify icon indicating copy to clipboard operation
notify copied to clipboard

PollWatcher may deadlock when user try change watch path in event_handler

Open visig9 opened this issue 3 years ago • 4 comments

System details

I believe this one affect all environment.

What you did

//! # Dependencies
//!
//! ```toml
//! [dependencies]
//! notify = "5.0.0-pre.15"
//! ```
//!
//! # Test
//!
//! ```sh
//! # in first terminal
//! touch mew.txt
//! cargo run
//!
//! # in second terminal
//! touch mew.txt  # try repeat multiple time
//! ```

use notify::{PollWatcher, RecursiveMode, Watcher};
use std::{
    sync::{Arc, Mutex},
    thread,
    time::Duration,
};

fn main() {
    // pass watcher into event handler.
    let arc_watcher_1: Arc<Mutex<Option<PollWatcher>>> = Default::default();
    let arc_watcher_2 = Arc::clone(&arc_watcher_1);

    // start watcher
    let mut watcher = PollWatcher::with_config(
        move |event| {
            println!("{event:?}");

            // try dead lock PollWatcher's internal loop.
            //
            // Scenario: User may try to add another watch path in event handler.
            if let Some(watcher) = arc_watcher_2.lock().unwrap().as_mut() {
                watcher
                    .watch("bow.txt".as_ref(), RecursiveMode::NonRecursive)
                    .unwrap();
            }

            // if this line printed, no deadlock.
            println!("add new watch path in event handler, successful")
        },
        notify::poll::PollWatcherConfig {
            poll_interval: Duration::from_secs(1),
            compare_contents: true,
        },
    )
    .unwrap();

    // watch a location.
    watcher
        .watch("mew.txt".as_ref(), RecursiveMode::NonRecursive)
        .unwrap();

    arc_watcher_1.lock().unwrap().replace(watcher);

    thread::park();
}

What you expected

No deadlock.

What happened

Deadlocked.

Investigate

When user try to change current watching paths, the watcher will take the lock from internal data, See:

  • https://github.com/notify-rs/notify/blob/0245625999e728ed3ce8dc6fdd5b6a7ad17e6510/src/poll.rs#L314
  • https://github.com/notify-rs/notify/blob/0245625999e728ed3ce8dc6fdd5b6a7ad17e6510/src/poll.rs#L377

But when event_handler be called, this lock are already hold by internal loop, so we call lock() twice at single thread, deadlock.


I may fix it (wait until #409 merged) by add a dedicated thread as event consumer, but after this fix, we will have two thread for each PollWatcher, just little heavy :(

visig9 avatar May 22 '22 12:05 visig9

Is this fixed by #409 ?

0xpr03 avatar Aug 10 '22 10:08 0xpr03

This bug not fixed yet. #409 only include refactoring, almost all behavior are the same.

Recently I have no time to deal this one. If you want to fix it just take it :)

visig9 avatar Aug 10 '22 12:08 visig9

On which system did you get this to deadlock ?

0xpr03 avatar Aug 10 '22 12:08 0xpr03

I'm tested within debian bullseye. But pretty sure this bug affect all platform.

The deadlock will happen when user try to change (watch / unwatch) watcher in its event_handler. The API design allow user to do that but it cannot work properly.

visig9 avatar Aug 10 '22 23:08 visig9