From 3d1b888bccdd27a28f9f12a4e89288afe1ee493a Mon Sep 17 00:00:00 2001 From: Georg Pichler Date: Thu, 10 Oct 2024 19:51:40 +0200 Subject: [PATCH] [inotify] Use of `select.poll()` instead of deprecated `select.select()` (#1078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Use `select.poll()` if available. Details: As stated in the `select()` man page: ``` 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. ``` This can lead to `ValueError: filedescriptor out of range in select()` when using watchdog. Following the advice of the `select()` man page, we use `select.poll()` instead, if available. The call to `select()` used as a fallback. * Add changelog entry for `select.poll()` usage. * Add a unit-test to ensure that we can handle file descriptors >1024. --- changelog.rst | 2 +- src/watchdog/observers/inotify_c.py | 14 ++++++++-- tests/test_inotify_c.py | 43 ++++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/changelog.rst b/changelog.rst index 4546f7df..61173137 100644 --- a/changelog.rst +++ b/changelog.rst @@ -8,7 +8,7 @@ Changelog 2024-xx-xx • `full history `__ -- +- [inotify] Use of ``select.poll()`` instead of deprecated ``select.select()``, if available. (`#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 bb2b1aa6..023609d5 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 diff --git a/tests/test_inotify_c.py b/tests/test_inotify_c.py index 8d4b59d4..32bfbaa3 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)