Watchdog: Calling inotify_init() on each watch

[Naatan]

I was getting the error OSError: inotify instance limit reached with only about 50 watchers open, which seemed weird. So I started digging and found that inotify_init() is called for pretty much each watcher: https://github.com/gorakhargosh/watchdog/blob/master/src/watchdog/observers/inotify_c.py#L163

I verified this by adding a print statement and seeing it print out my message for each watch that was being added. I then tried the following workaround:

--- a/watchdog/src/watchdog/observers/inotify_c.py
+++ b/watchdog/src/watchdog/observers/inotify_c.py
@@ -158,12 +158,19 @@ class Inotify(object):
         ``True`` if subdirectories should be monitored; ``False`` otherwise.
     """

+    _inotify_fd = None
+
     def __init__(self, path, recursive=False, event_mask=WATCHDOG_ALL_EVENTS):
         # The file descriptor associated with the inotify instance.
-        inotify_fd = inotify_init()
-        if inotify_fd == -1:
-            Inotify._raise_error()
-        self._inotify_fd = inotify_fd
+        if not Inotify._inotify_fd:
+            inotify_fd = inotify_init()
+
+            if inotify_fd == -1:
+                Inotify._raise_error()
+
+            Inotify._inotify_fd = inotify_fd
+
+        self._inotify_fd = Inotify._inotify_fd
         self._lock = threading.Lock()

         # Stores the watch descriptor for a given path.

This seemed to work, in that watchers are added without also increasing the number of inotify instances beyond the one that is required. However this breaks the actual events as well as removing the watchers, giving errors such as

Exception in thread Thread-21:
Traceback (most recent call last):
  File "**/dist/python/lib/python2.7/threading.py", line 808, in __bootstrap_inner
    self.run()
  File "**/dist/bin/python/komodo/watchdog/observers/inotify_buffer.py", line 57, in run
    inotify_events = self._inotify.read_events()
  File "**/dist/bin/python/komodo/watchdog/observers/inotify_c.py", line 302, in read_events
    wd_path = self._path_for_wd[wd]
KeyError: 4

I'm hesitant to dig much further at this time because being as I am not familiar with this codebase I do not fully understand the implications. It seems obvious to me though that adding an instance for each watch is not what you should be doing though, hopefully this can get fixed relatively easily but I'll have to defer to people who know this codebase thoroughly.

[tamland]

I'm not the original author of that, but I believe the Inotify class was done like this in an attempt to create an object oriented wrapper around the inotify api. It encapsulate the whole api, that's why an inotify file descriptor is created for every instance. Apparently not good idea if you plan to create many of them. However, for me the inotify instance limit is set to 128. I don't know why you only got 50. Maybe you have other applications creating inotify instances..

The error you see in read_events is because if you simply share the file descriptor, every instance of Inotify will read from the same file descriptor (see inotify_c.py#L284) and receive events that does not belong to it.

To use only one file descriptor you will need to make the event reading static too, then distribute the events to the watch where it belongs somehow.

[Naatan]

Note the "whole Inotify api" consists of about 3 functions, so not sure how valid this object oriented approach is. Regardless this doesn't address the problem; watchdog is adding instances for each watcher; this is a bug no matter what design principle you apply to it.

If there is currently no active developer thoroughly familiar with this code I'd be happy to play further with it to try and come up with a proper fix, but I am mainly worried that this would require a ton of rewriting as it was obviously never designed to work with a single inotify instance (even though it should). Seems to me that this was implemented without a thorough understanding of how inotify works. Now I don't by any stretch claim to have a thorough understanding of inotify (on the contrary), but it seems obvious to me at this point that you should not be adding an inotify instance for each watcher.

[tamland]

Besides the unfortunate low default limit I don't think there is anything that says you must only create one inotify instance. There's mismatch of APIs. watchdog is designed around watching separate _trees_, while inotify aground single directories or files. Feel free to try to suggest a solution. Alternatively if you don't mind managing inotify watches yourself, implement an EventEmitter that don't create multiple inotify instances, but you'll lose the ability to have different event handles for different trees.