search

gorakhargosh / watchdog

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

Properly clean up threads when stopping Inotify. Improve Eventlet tests. #1070

Closed ethan-vanderheijden closed 2 months ago

ethan-vanderheijden commented 2 months ago

Fixes #1045 Fixes #679

New Eventlet release means it's time to fix these bugs! Actually, it turns out we don't need the latest Eventlet version to get this working, so I could have put this PR out months ago...

I made a couple minor improvements:

  1. Inotify cleans up its threads more reliably
    • Now, it reads the inotify file descriptor and an extra file descriptor simultaneously. When we want to shutdown Inotify, we can write to that second file descriptor and guarantee that any thread waiting for data is woken up.
  2. SkipRepeatsQueue is now compatible with both the standard Python Queue and Eventlet's monkey patched version
  3. Improved tests related to Eventlet and moved them into their own file
    • All tests in pytest are run inside the same Python interpreter, so if you do something that modifies global state, like calling eventlet.monkey_patch(), you run the risk of interfering with other tests. To be safe, I moved tests with Eventlet monkey patching into tests/isolated/ and ran them in a separate Python interpreter with Popen.

Let me know if there's anything else that I need to do

ethan-vanderheijden commented 2 months ago

It looks like Eventlet support is only possible on Linux with Inotify.

BSD uses Kqueue, which Evenlet doesn't support. In fact, Evenlet deletes kqueue from the select module entirely (see).

The problem with Windows is that the call to ReadDirectoryChangesW is blocking, and since that call is made through the C API, it can't be monkey patched by Eventlet. I think it's possible to call ReadDirectoryChangesW asynchronously and set up a callback function, but frankly, I'm not very familiar with Windows API.

BoboTiG commented 2 months ago

That's awesome, thank you @ethan-vanderheijden !

I'll merge and deal myself with typing issues ;)

BoboTiG commented 1 month ago

It's part of the version 5.0.3 :champagne: