Change in vdsm[ovirt-3.6]: supervdsm: failed validateAccess leaves pipes open

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Mon Jan 18 14:45:33 UTC 2016


Piotr Kliczewski has uploaded a new change for review.

Change subject: supervdsm: failed validateAccess leaves pipes open
......................................................................

supervdsm: failed validateAccess leaves pipes open

Whenever we run validateAccess and it fails with "Permission denied" we
leak file descriptors. We need to make sure to close child and parent
pipes.


Change-Id: I6356837686c274389e92e828143cce3a5eeff564
Signed-off-by: pkliczewski <piotr.kliczewski at gmail.com>
Bug-url: https://bugzilla.redhat.com/1271575
---
M vdsm/supervdsmServer
1 file changed, 27 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/65/52365/1

diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 247b599c..bc7836d 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -31,6 +31,8 @@
 import signal
 import logging
 import logging.config
+
+from contextlib import closing
 from vdsm.infra import sigutils
 from vdsm.infra import zombiereaper
 
@@ -264,37 +266,38 @@
             pipe.recv()
 
         pipe, hisPipe = Pipe()
-        proc = Process(target=child, args=(hisPipe,))
-        proc.start()
+        with closing(pipe), closing(hisPipe):
+            proc = Process(target=child, args=(hisPipe,))
+            proc.start()
 
-        needReaping = True
-        try:
-            if not safe_poll(pipe, RUN_AS_TIMEOUT):
-                try:
+            needReaping = True
+            try:
+                if not safe_poll(pipe, RUN_AS_TIMEOUT):
+                    try:
 
-                    os.kill(proc.pid, signal.SIGKILL)
-                except OSError as e:
-                    # Don't add to zombiereaper of PID no longer exists
-                    if e.errno == errno.ESRCH:
-                        needReaping = False
-                    else:
-                        raise
+                        os.kill(proc.pid, signal.SIGKILL)
+                    except OSError as e:
+                        # Don't add to zombiereaper of PID no longer exists
+                        if e.errno == errno.ESRCH:
+                            needReaping = False
+                        else:
+                            raise
 
-                raise Timeout()
+                    raise Timeout()
 
-            res, err = pipe.recv()
-            pipe.send("Bye")
-            proc.terminate()
+                res, err = pipe.recv()
+                pipe.send("Bye")
+                proc.terminate()
 
-            if err is not None:
-                raise err
+                if err is not None:
+                    raise err
 
-            return res
+                return res
 
-        finally:
-            # Add to zombiereaper if process has not been waited upon
-            if proc.exitcode is None and needReaping:
-                zombiereaper.autoReapPID(proc.pid)
+            finally:
+                # Add to zombiereaper if process has not been waited upon
+                if proc.exitcode is None and needReaping:
+                    zombiereaper.autoReapPID(proc.pid)
 
     @logDecorator
     def validateAccess(self, user, groups, *args, **kwargs):


-- 
To view, visit https://gerrit.ovirt.org/52365
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6356837686c274389e92e828143cce3a5eeff564
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>


More information about the vdsm-patches mailing list