Fix garbage collection of Notifier

[blueyed]

trafficstars

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 https://github.com/seb-m/pyinotify/issues/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]

Rebased on master.

@seb-m What do you think about it?

[seb-m]

Hi @blueyed,

I still don't have an opinion about it.

I must say I didn't have time to review it and think about it carefully and this is the kind of structural change I don't want to commit lightly.

[phillipberndt]

FYI, I just ran into this issue and thought of a similar fix. But I think that closing the fd from Notifier is the wrong thing to do (and has been in the first place, it's not introduced by this PR); the fd is owned by WatchManager, hence it should not be closed until the WatchManager is destroyed. I'd therefore suggest an additional change: Remove the call to os.close from Notifier.stop, and add a WatchManager.__del__ method that calls WatchManager.close.