pyroute2 icon indicating copy to clipboard operation
pyroute2 copied to clipboard

NSPopen leaks write FDs on release

Open salv-orlando opened this issue 5 years ago • 1 comments

This is apparently very similar to issue #167

The close operation for Queues invoked on release() seem to close only the reader socket. The writer socket is instead not closed. This is probably more an issue of the underlying multiprocess library.

The following patch seems a decent workaround (at least works for me). If you find it useful, I can morph it into a PR (and maybe remove the log statement with some better exception handling)

diff --git a/pyroute2/netns/process/proxy.py b/pyroute2/netns/process/proxy.py
index b73f8a05..3805e804 100644
--- a/pyroute2/netns/process/proxy.py
+++ b/pyroute2/netns/process/proxy.py
@@ -7,7 +7,7 @@ all, but it is required to have a reasonable network
 namespace support.
 
 '''
-
+import logging
 import sys
 import fcntl
 import types
@@ -19,6 +19,8 @@ from pyroute2.netns import setns
 from pyroute2.common import metaclass
 from pyroute2.netns.process import MetaPopen
 
+log = logging.getLogger(__name__)
+
 
 def _handle(result):
     if result['code'] == 500:
@@ -270,6 +272,8 @@ class NSPopen(ObjNS):
         self.server.start()
         response = self.channel_in.get()
         if isinstance(response, Exception):
+            log.warning("Child process raised: %s. FDs may be leaked",
+                        response)
             self.server.join()
             raise response
         else:
@@ -289,6 +293,8 @@ class NSPopen(ObjNS):
             self.channel_out.close()
             self.channel_in.close()
             self.server.join()
+            self.channel_out._writer.close()
+            self.channel_in._writer.close()
 
     def __dir__(self):
         return list(self.api.keys()) + ['release']

salv-orlando avatar Jul 17 '19 06:07 salv-orlando

Yep, please post a PR. The proposal looks reasonable.

svinota avatar Aug 20 '19 11:08 svinota