Repository: gorakhargosh/watchdog

Issue:
Use of `select.select()` might trigger a `ValueError` if fd>1024 is passed
I was using watchdog extensively in a quite large application with many threads. It would occasionally result in `ValueError: filedescriptor out of range in select()` exceptions.

After some searching, I believe that the culprit is indeed the `select.select()` call. On Linux the man page says
>WARNING: select() can monitor only file descriptors numbers that are less than FD_SETSIZE (1024)—an unreasonably low limit for many modern applications—and this limitation will not change. All modern applications should instead use poll(2) or epoll(7), which do not suffer this limitation.

I tried to move to `select.poll()` in #1078. Maybe there is a better approach than manually checking `hasattr(select, 'poll')`. Let me know if this works or any other options are preferred. In order to avoid regressions, it might be wise to add a test that opens >1024 file descriptors and then uses watchdog or something along those lines. If this is required, I could take a stab at it.


Discussion:
BoboTiG: Nice, thank you @g-pichler !

Would you mond adding a line in the changelog too, please?
BoboTiG: That's OK on my side. LMK when you are ready for the merge.
BoboTiG: Out of curiosity, any pros/cons to prefer `epoll()`?
g-pichler: I have to be honest, I did not look into `epoll()` after my issue was resolved using `poll()`. I'm also not familiar with polling and did not want to invest more time, considering I had a workable solution and the man page was pointing at `poll()` as a suitable replacement.

From my side, you can merge at your convenience. I think a MacOS unit-test keeps failing from time to time, but I believe this might be a timing issue in the CI/CD.

Patch:
diff --git a/changelog.rst b/changelog.rst
index 4546f7d..6117313 100644
--- a/changelog.rst
+++ b/changelog.rst
@@ -8,7 +8,7 @@ Changelog
 
 2024-xx-xx • `full history <https://github.com/gorakhargosh/watchdog/compare/v5.0.3...HEAD>`__
 
--
+- [inotify] Use of ``select.poll()`` instead of deprecated ``select.select()``, if available. (`#1078 <https://github.com/gorakhargosh/watchdog/pull/1078>`__)
 - Thanks to our beloved contributors: @BoboTiG, @
 
 5.0.3
diff --git a/src/watchdog/observers/inotify_c.py b/src/watchdog/observers/inotify_c.py
index bb2b1aa..023609d 100644
--- a/src/watchdog/observers/inotify_c.py
+++ b/src/watchdog/observers/inotify_c.py
@@ -9,8 +9,8 @@
 import struct
 import threading
 from ctypes import c_char_p, c_int, c_uint32
-from functools import reduce
-from typing import TYPE_CHECKING
+from functools import partial, reduce
+from typing import TYPE_CHECKING, Any, Callable
 
 from watchdog.utils import UnsupportedLibcError
 
@@ -153,6 +153,14 @@ def __init__(self, path: bytes, *, recursive: bool = False, event_mask: int | No
         self._waiting_to_read = True
         self._kill_r, self._kill_w = os.pipe()
 
+        if hasattr(select, "poll"):
+            self._poller = select.poll()
+            self._poller.register(self._inotify_fd, select.POLLIN)
+            self._poller.register(self._kill_r, select.POLLIN)
+            self._poll: Callable[[], Any] = partial(self._poller.poll)
+        else:
+            self._poll = partial(select.select, (self._inotify_fd, self._kill_r))
+
         # Stores the watch descriptor for a given path.
         self._wd_for_path: dict[bytes, int] = {}
         self._path_for_wd: dict[int, bytes] = {}
@@ -292,7 +300,7 @@ def _recursive_simulate(src_path: bytes) -> list[InotifyEvent]:
 
                     self._waiting_to_read = True
 
-                select.select([self._inotify_fd, self._kill_r], [], [])
+                self._poll()
 
                 with self._lock:
                     self._waiting_to_read = False


Test patch:
diff --git a/tests/test_inotify_c.py b/tests/test_inotify_c.py
index 8d4b59d..32bfbaa 100644
--- a/tests/test_inotify_c.py
+++ b/tests/test_inotify_c.py
@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from contextlib import ExitStack
+
 import pytest
 
 from watchdog.utils import platform
@@ -64,6 +66,24 @@ def fakeselect(read_list, *args, **kwargs):
             return [inotify_fd], [], []
         return select_bkp(read_list, *args, **kwargs)
 
+    poll_bkp = select.poll
+
+    class Fakepoll:
+        def __init__(self):
+            self._orig = poll_bkp()
+            self._fake = False
+
+        def register(self, fd, *args, **kwargs):
+            if fd == inotify_fd:
+                self._fake = True
+                return None
+            return self._orig.register(fd, *args, **kwargs)
+
+        def poll(self, *args, **kwargs):
+            if self._fake:
+                return None
+            return self._orig.poll(*args, **kwargs)
+
     os_read_bkp = os.read
 
     def fakeread(fd, length):
@@ -101,8 +121,9 @@ def inotify_rm_watch(fd, wd):
     mock4 = patch.object(inotify_c, "inotify_add_watch", new=inotify_add_watch)
     mock5 = patch.object(inotify_c, "inotify_rm_watch", new=inotify_rm_watch)
     mock6 = patch.object(select, "select", new=fakeselect)
+    mock7 = patch.object(select, "poll", new=Fakepoll)
 
-    with mock1, mock2, mock3, mock4, mock5, mock6:
+    with mock1, mock2, mock3, mock4, mock5, mock6, mock7:
         start_watching(path=p(""))
         # Watchdog Events
         for evt_cls in [DirCreatedEvent, DirDeletedEvent] * 2:
@@ -168,3 +189,23 @@ def test_event_equality(p: P) -> None:
     assert event1 == event2
     assert event1 != event3
     assert event2 != event3
+
+
+def test_select_fd(p: P, event_queue: TestEventQueue, start_watching: StartWatching) -> None:
+    # We open a file 2048 times to ensure that we exhaust 1024 file
+    # descriptors, the limit of a select() call.
+    path = p("new_file")
+    with open(path, "a"):
+        pass
+    with ExitStack() as stack:
+        for _i in range(2048):
+            stack.enter_context(open(path))
+
+        # Watch this file for deletion (copied from `test_watch_file`)
+        path = p("this_is_a_file")
+        with open(path, "a"):
+            pass
+        start_watching(path=path)
+        os.remove(path)
+        event, _ = event_queue.get(timeout=5)
+        assert repr(event)