Merge pull request #643 from dfaust/release-debouncer-full-0.3.2

Prepare release of debouncer full 0.3.2

Author: dfaust

CHANGELOG.md

@@ -3,6 +3,12 @@
 v5 maintenance branch is on `v5_maintenance` after `5.2.0`  
 v4 commits split out to branch `v4_maintenance` starting with `4.0.16`
 
+## debouncer-full 0.3.2 (2024-09-29)
+
+- FIX: ordering of debounced events could lead to a panic with Rust 1.81.0 and above [#636]
+
+[#636]: https://github.com/notify-rs/notify/issues/636
+
 ## debouncer-full 0.3.1 (2023-08-21)
 
 - CHANGE: remove serde binary experiment opt-out after it got removed [#530]

notify-debouncer-full/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "notify-debouncer-full"
-version = "0.3.1"
+version = "0.3.2"
 edition = "2021"
 rust-version = "1.60"
 description = "notify event debouncer optimized for ease of use"

notify-debouncer-full/src/lib.rs

@@ -57,9 +57,9 @@
 //! - `crossbeam` enabled by default, adds [`DebounceEventHandler`](DebounceEventHandler) support for crossbeam channels.
 //!   Also enables crossbeam-channel in the re-exported notify. You may want to disable this when using the tokio async runtime.
 //! - `serde` enables serde support for events.
-//! 
+//!
 //! # Caveats
-//! 
+//!
 //! As all file events are sourced from notify, the [known problems](https://docs.rs/notify/latest/notify/#known-problems) section applies here too.
 
 mod cache;
@@ -69,7 +69,8 @@ mod debounced_event;
 mod testing;
 
 use std::{
-    collections::{HashMap, VecDeque},
+    cmp::Reverse,
+    collections::{BinaryHeap, HashMap, VecDeque},
     path::PathBuf,
     sync::{
         atomic::{AtomicBool, Ordering},
@@ -249,17 +250,7 @@ impl<T: FileIdCache> DebounceDataInner<T> {
 
         self.queues = queues_remaining;
 
-        // order events for different files chronologically, but keep the order of events for the same file
-        events_expired.sort_by(|event_a, event_b| {
-            // use the last path because rename events are emitted for the target path
-            if event_a.paths.last() == event_b.paths.last() {
-                std::cmp::Ordering::Equal
-            } else {
-                event_a.time.cmp(&event_b.time)
-            }
-        });
-
-        events_expired
+        sort_events(events_expired)
     }
 
     /// Returns all currently stored errors
@@ -654,6 +645,49 @@ pub fn new_debouncer<F: DebounceEventHandler>(
     )
 }
 
+fn sort_events(events: Vec<DebouncedEvent>) -> Vec<DebouncedEvent> {
+    let mut sorted = Vec::with_capacity(events.len());
+
+    // group events by path
+    let mut events_by_path: HashMap<_, VecDeque<_>> =
+        events.into_iter().fold(HashMap::new(), |mut acc, event| {
+            acc.entry(event.paths.last().cloned().unwrap_or_default())
+                .or_default()
+                .push_back(event);
+            acc
+        });
+
+    // push events for different paths in chronological order and keep the order of events with the same path
+
+    let mut min_time_heap = events_by_path
+        .iter()
+        .map(|(path, events)| Reverse((events[0].time, path.clone())))
+        .collect::<BinaryHeap<_>>();
+
+    while let Some(Reverse((min_time, path))) = min_time_heap.pop() {
+        // unwrap is safe because only paths from `events_by_path` are added to `min_time_heap`
+        // and they are never removed from `events_by_path`.
+        let events = events_by_path.get_mut(&path).unwrap();
+
+        let mut push_next = false;
+
+        while events.front().map_or(false, |event| event.time <= min_time) {
+            // unwrap is safe beause `pop_front` mus return some in order to enter the loop
+            let event = events.pop_front().unwrap();
+            sorted.push(event);
+            push_next = true;
+        }
+
+        if push_next {
+            if let Some(event) = events.front() {
+                min_time_heap.push(Reverse((event.time, path)));
+            }
+        }
+    }
+
+    sorted
+}
+
 #[cfg(test)]
 mod tests {
     use std::{fs, path::Path};
@@ -702,7 +736,9 @@ mod tests {
             "emit_close_events_only_once",
             "emit_modify_event_after_close_event",
             "emit_needs_rescan_event",
-            "read_file_id_without_create_event"
+            "read_file_id_without_create_event",
+            "sort_events_chronologically",
+            "sort_events_with_reordering"
         )]
         file_name: &str,
     ) {

notify-debouncer-full/test_cases/sort_events_chronologically.hjson

@@ -0,0 +1,42 @@
+{
+    state: {
+        queues: {
+            /watch/file-1: {
+                events: [
+                    { kind: "create-any", paths: ["*"], time: 2 }
+                    { kind: "modify-any", paths: ["*"], time: 3 }
+                ]
+            }
+            /watch/file-2: {
+                events: [
+                    { kind: "create-any", paths: ["*"], time: 1 }
+                    { kind: "modify-any", paths: ["*"], time: 4 }
+                ]
+            }
+        }
+    }
+    expected: {
+        queues: {
+            /watch/file-1: {
+                events: [
+                    { kind: "create-any", paths: ["*"], time: 2 }
+                    { kind: "modify-any", paths: ["*"], time: 3 }
+                ]
+            }
+            /watch/file-2: {
+                events: [
+                    { kind: "create-any", paths: ["*"], time: 1 }
+                    { kind: "modify-any", paths: ["*"], time: 4 }
+                ]
+            }
+        }
+        events: {
+            long: [
+                { kind: "create-any", paths: ["/watch/file-2"], time: 1 }
+                { kind: "create-any", paths: ["/watch/file-1"], time: 2 }
+                { kind: "modify-any", paths: ["/watch/file-1"], time: 3 }
+                { kind: "modify-any", paths: ["/watch/file-2"], time: 4 }
+            ]
+        }
+    }
+}

notify-debouncer-full/test_cases/sort_events_with_reordering.hjson

@@ -0,0 +1,42 @@
+{
+    state: {
+        queues: {
+            /watch/file-1: {
+                events: [
+                    { kind: "create-any", paths: ["*"], time: 2 }
+                    { kind: "modify-any", paths: ["*"], time: 3 }
+                ]
+            }
+            /watch/file-2: {
+                events: [
+                    { kind: "rename-to", paths: ["*"], time: 4 }
+                    { kind: "modify-any", paths: ["*"], time: 1 }
+                ]
+            }
+        }
+    }
+    expected: {
+        queues: {
+            /watch/file-1: {
+                events: [
+                    { kind: "create-any", paths: ["*"], time: 2 }
+                    { kind: "modify-any", paths: ["*"], time: 3 }
+                ]
+            }
+            /watch/file-2: {
+                events: [
+                    { kind: "rename-to", paths: ["*"], time: 4 }
+                    { kind: "modify-any", paths: ["*"], time: 1 }
+                ]
+            }
+        }
+        events: {
+            long: [
+                { kind: "create-any", paths: ["/watch/file-1"], time: 2 }
+                { kind: "modify-any", paths: ["/watch/file-1"], time: 3 }
+                { kind: "rename-to", paths: ["/watch/file-2"], time: 4 }
+                { kind: "modify-any", paths: ["/watch/file-2"], time: 1 }
+            ]
+        }
+    }
+}