IPRoute class leaks socket/pipe file descriptors

[veeagraham]

Apologies if this is a known issue/feature ...

We've found that if you create an instance of IPRoute and this instance is later destroyed, the code "leaks" socket/pipe file descriptors.

We can see this by listing the /proc/<the pid>/fd directory.

We can cure this by explicitly creating a __del__ method on our classes and calling the IPRoute instance .close() method.

However, I've not seen this feature (or warning?) documented anywhere ...

This is with pyroute2 0.5.3 under Linux 4.15

Example to reproduce:

#!/usr/bin/env python

from __future__ import print_function

import os
import time

from pyroute2 import IPRoute


class MyClass:
    def _print_files_count(self, caller):
        count = len([name for name in os.listdir('/proc/self/fd')])
        print("{}: files = {}".format(caller, count))

    def __init__(self):
        self._print_files_count("ctor")

        self._ipr = IPRoute()

    def __del__(self):
        # Uncomment the following to "cure" the socket+pipe leak ...
        # self._ipr.close()

        self._print_files_count("dtor")


if __name__ == "__main__":
    for i in range(20):
        ipr = MyClass()

        del ipr

        time.sleep(1)

[svinota]

This issue is documented, but obviously not clear enough — otherwise you wouldn't create the ticket :)

Yep, the idea to call IPRoute.close() from __del__() looks reasonable, I'm to try.

[crosser]

I think that using the destructor for resource management is generally discouraged on the grounds that the time when it is called is unpredictable (and may never happen at all)...

[svinota]

But the problem stays. If one destroys (del) an IPRoute object w/o calling .close() explicitly, the fds — implicitly opened by __init__() — remain opened. I'm to update the docs to emphasize the importance of the .close(), but placing last checks to __del__() sounds not as bad. Not relying on these checks but run them just in case, when and if GC collects dead objects.

[crosser]

You are right, it's better than just losing the descriptor altogether. An alternative would be to raise an exception, but that would be perhaps too harsh.