notify icon indicating copy to clipboard operation
notify copied to clipboard

notify-debouncer-full panic on Linux with inotify

Open kevinchevalier opened this issue 9 months ago • 2 comments

If no paths are included on an Event, notify-debouncer-full panics due to this line. https://github.com/notify-rs/notify/blob/main/notify-debouncer-full/src/lib.rs#L274

It appear that INotifyWatcher on Linux sometimes includes events with no paths.

This is rare, but I'm seeing it happen in telemetry 50 times a day.

I could take a crack at addressing this issue, but I'm not sure whether the fix would be to:

  • Suppress all events from INotifyWatcher that do not include paths. OR
  • Make notify-debouncer-full handle events safely that don't include paths.

kevinchevalier avatar Mar 04 '25 20:03 kevinchevalier

Thanks for filing this issue. Do you happen to know what kind of events these are? I don't know if we should just ignore them, but the issue you found in notify-debouncer-full is definitely a bug. I assumed that all events had a path. I guess we should just ignore those without paths then.

dfaust avatar Mar 04 '25 20:03 dfaust

I'm sorry to say that I don't know what kind of events these are.

Looking at the watcher code, it looks like it could be almost any kind of event if the working directory was not tracked by the watcher.

https://github.com/notify-rs/notify/blob/main/notify/src/inotify.rs#L221

I'll create a first draft for a PR to ignore these events.

kevinchevalier avatar Mar 04 '25 21:03 kevinchevalier

I think ignoring is a mistake (IMHO).

There is one case when event without paths is expected - when inotify queue is overflowed. User should be able to handle this, because it means some events might be lost.

There is a good explanation on Flag enum.

There is inotify code, which triggers this event

riberk avatar Jul 15 '25 11:07 riberk

Oh, I was wrong, sorry about that.

The checking of need_rescan is before the ignoring an event in the PR.

But, after some investigation, I think it might happen because path doesn't have the taken descriptor. Some code is related to that

I'm going to try reproducing it tomorrow because my task is strongly related to the INotifyWatcher and I think the problem either the race condition (descriptor was deleted from paths but event has already been got) or some event types from inotify was ignored but related.

It looks like the first is likely to be true.

I do not know, if I'll be success or not, because the only way to reproduce I can imagine is

  • Watch a dir
  • -- Race condition start
  • Unwatch the dir
  • Got an event
  • -- Race condition end

If unwatch is performed before handling inotify event, the behavior described in the issue will happen.

Anyway, I think it is a bug in notify_debouncer_full AND in inotify.

  • Debouncer should ignore that kind of events, because it doesn't know what should be done with them.
  • INotifyWatcher should ignore that events if it is definitely race condition.

If I am able to understand what the problem exactly is I'll create a PR that fixes notify_debouncer_full and inotify

riberk avatar Jul 15 '25 19:07 riberk

OK, it was races

This test did not pass before the fix

riberk avatar Jul 16 '25 09:07 riberk

Thanks for the fix.

kevinchevalier avatar Jul 21 '25 14:07 kevinchevalier