Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide hint when running out of watches / instances; leak via react.py #95

Open
blueyed opened this issue Mar 16, 2015 · 4 comments · May be fixed by #96
Open

Provide hint when running out of watches / instances; leak via react.py #95

blueyed opened this issue Mar 16, 2015 · 4 comments · May be fixed by #96

Comments

@blueyed
Copy link
Contributor

blueyed commented Mar 16, 2015

I've seen the following errors (via Django):

ERROR:pyinotify:add_watch: cannot watch /path/to/file/ WD=-1, Errno=No space left on device (ENOSPC)

This can be caused when running out of inotify instances or watches (ref).

I could track it down to react causing it when it is reloading, at least the number of instances gets increased then.

I've not taken a closer look yet, but it's probably rather trivial for you to spot it: https://github.com/copton/react/blob/master/react.py

Anyway, the issue with pyinotify is that the exception / error message could be a bit more informative maybe, instead of only providing the ENOSPC reference.

The limits that might be relevant are:

% for i in /proc/sys/fs/inotify/*; do echo "${i:t}: $(<$i)"; done
max_queued_events: 16384
max_user_instances: 128
max_user_watches: 524288

Maybe pyinotify can help in use cases like this, too, by providing some garbarge collection / cleanup automatically?

@blueyed
Copy link
Contributor Author

blueyed commented Mar 16, 2015

It turns out that notifier.stop() needs to be called on reload: https://github.com/copton/react/pull/1/files

Would it be possible for pyinotify to handle this, when the old notifier reference gets overwritten / garbage collected?

blueyed added a commit to blueyed/pyinotify that referenced this issue Mar 16, 2015
This uses a weak reference to the notifier in _SysProcessEvent and adds
a `__del__` method for Notifier.

This prevents pyinotify from leaking inotify file descriptors
(instances) and its watches.

Fixes seb-m#95

I have used the following script to test it:

    from __future__ import print_function

    import gc
    import os

    from pyinotify import Notifier, WatchManager

    def has_inotify_fds():
        "Does the current process have inotify fds?"
        r = 0
        for x in os.walk('/proc/{}/fd'.format(os.getpid())):
            for y in x[2]:
                path = os.path.join(x[0], y)
                if os.path.exists(path):
                    if 'inotify' in os.path.join(os.readlink(path)):
                        r += 1
        return r

    gc.disable()

    wm = WatchManager()
    n = Notifier(wm)
    assert has_inotify_fds() == 1

    # Stopping multiple times should work:
    n.stop()
    assert has_inotify_fds() == 0
    n.stop()

    gc.collect()
    # XXX: needs a new WatchManager, because the previous fd gets closed by stop!
    wm = WatchManager()
    n = Notifier(wm)
    assert has_inotify_fds() == 1

    # Collect anything.
    gc.collect()
    assert n in gc.get_objects()

    n = None
    assert gc.garbage == []
    assert not any(isinstance(x, Notifier) for x in gc.garbage)
    assert not any(isinstance(x, Notifier) for x in gc.get_objects())

    assert has_inotify_fds() == 0
@blueyed blueyed linked a pull request Mar 16, 2015 that will close this issue
blueyed added a commit to blueyed/pyinotify that referenced this issue Jun 6, 2015
This uses a weak reference to the notifier in _SysProcessEvent and adds
a `__del__` method for Notifier.

This prevents pyinotify from leaking inotify file descriptors
(instances) and its watches.

Fixes seb-m#95

I have used the following script to test it:

    from __future__ import print_function

    import gc
    import os

    from pyinotify import Notifier, WatchManager

    def has_inotify_fds():
        "Does the current process have inotify fds?"
        r = 0
        for x in os.walk('/proc/{}/fd'.format(os.getpid())):
            for y in x[2]:
                path = os.path.join(x[0], y)
                if os.path.exists(path):
                    if 'inotify' in os.path.join(os.readlink(path)):
                        r += 1
        return r

    gc.disable()

    wm = WatchManager()
    n = Notifier(wm)
    assert has_inotify_fds() == 1

    # Stopping multiple times should work:
    n.stop()
    assert has_inotify_fds() == 0
    n.stop()

    gc.collect()
    # XXX: needs a new WatchManager, because the previous fd gets closed by stop!
    wm = WatchManager()
    n = Notifier(wm)
    assert has_inotify_fds() == 1

    # Collect anything.
    gc.collect()
    assert n in gc.get_objects()

    n = None
    assert gc.garbage == []
    assert not any(isinstance(x, Notifier) for x in gc.garbage)
    assert not any(isinstance(x, Notifier) for x in gc.get_objects())

    assert has_inotify_fds() == 0
@timabbott
Copy link

timabbott commented Jun 13, 2019

Is there any chance of this leak issue being resolved? There's a lot of issues around the Internet resulting from pyinotify both leaking watched files and also giving this confusing ENOSPC error message. See e.g. https://code.djangoproject.com/ticket/23523 or zulip/zulip#12565.

@blueyed
Copy link
Contributor Author

blueyed commented Jun 13, 2019

@timabbott
can you try #96?

@timabbott
Copy link

I'll try to test that out; because I don't have a consistent reproducer (it starts happening after a few days of use), I'm not sure I'll be able to have a clear "this fixed it" or not story until after a few weeks of use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants