gorakhargosh / watchdog

Python library and shell utilities to monitor filesystem events.
http://packages.python.org/watchdog/
Apache License 2.0
6.53k stars 695 forks source link

Threads not cleaning up when closing Observer if using Eventlet. #1045

Closed ethan-vanderheijden closed 2 weeks ago

ethan-vanderheijden commented 2 months ago

In that past, using Eventlet with Watchdog would cause your program to freeze indefinitely (#679). This was fixed with https://github.com/eventlet/eventlet/pull/975 (though they haven't published a new release yet).

However, there is a new problem. If you stop watchdog (using observer.stop()), the program will not exit because Watchdog fails to clean up its threads. The relevant code is in inotify_c.py.

The close method: https://github.com/gorakhargosh/watchdog/blob/9f23b599f369cdd873c0cf3dd9e69567234d78fb/src/watchdog/observers/inotify_c.py#L250-L258

The thread blocks while reading the file descriptor: https://github.com/gorakhargosh/watchdog/blob/9f23b599f369cdd873c0cf3dd9e69567234d78fb/src/watchdog/observers/inotify_c.py#L300-L303

Without Eventlet, if you call the close method, the call to inotify_rm_watch coincidentally causes the Inotify API to write output to the file descriptor. Specifically, it sends the event <InotifyEvent: src_path=b'...', wd=1, mask=IN_IGNORED, ...>. The thread which is blocking on os.read sees this output and wakes up. All of this happens before the file descriptor is closed with os.close(self._inotify_fd).

However, when you monkey patch with Evenlet, it converts all blocking os.read calls into non-blocking reads by first polling on the file descriptor and then reading when data is available. Now, when you call the close method, the call to inotify_rm_watch will still write data to the file descriptor, but the file descriptor is immediately closed before the reading thread is able to wake up and read the data. Consequently, the reading thread continue blocking and isn't cleaned up.

There are 2 fixes to this race condition:

  1. If you are certain that calling the close method will always write data to the file descriptor, you can defer closing the file descriptor until after the thread blocking on read has woken up.
  2. Instead of doing a blocking read, the thread can block on a select call that monitors both the file descriptor and another channel. When close is called, it writes to the secondary channel, guaranteeing that the thread is woken up.

I'd be happy to make a PR

BoboTiG commented 2 months ago

What an analysis @ethan-vanderheijden! Thanks a lot!

Actually, I would be happy to review a PR, so if you can then lets do this. Also I would love to see regression(s) test(s) included.