Nir Soffer has uploaded a new change for review.

Change subject: zombiereaper: Raise if used incorrectly
......................................................................

zombiereaper: Raise if used incorrectly

zombiereaper was trying to reap pid when it was not initialized
properly. This lead to slient failures and and leaking zombies processes
for the process lifetime.

In commit 0636b8607b (Revert "supervdsm: Add zombiereaper to supervdsm")
we removed zombiereaper from supervdsm, but there are still callers that
assume it is inizialized. This patch ensures callers will fail loudly if
zombiereaper was not initialzied.

The tests are more carefull now not change the Python process state when
importing tests from the test module.

Change-Id: Ib3a275a3f022c7b686ce07693b7c0b147762b043
Signed-off-by: Nir Soffer <nsof...@redhat.com>
---
M lib/vdsm/infra/zombiereaper/__init__.py
M lib/vdsm/infra/zombiereaper/tests.py
2 files changed, 19 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/37260/1

diff --git a/lib/vdsm/infra/zombiereaper/__init__.py 
b/lib/vdsm/infra/zombiereaper/__init__.py
index f85ec30..edc2afe 100644
--- a/lib/vdsm/infra/zombiereaper/__init__.py
+++ b/lib/vdsm/infra/zombiereaper/__init__.py
@@ -30,12 +30,15 @@
 import signal
 
 _trackedPids = set()
+_registered = False
 
 
 def autoReapPID(pid):
     """
     Register a PID to be auto-cleaned.
     """
+    if not _registered:
+        raise RuntimeError("zombiereaper is not registered for SIGCHLD")
     _trackedPids.add(pid)
     # SIGCHLD happend before we added the pid to the set
     _tryReap(pid)
@@ -60,7 +63,9 @@
     Set up the signal handler so that PIDs are reaped. Should be called once
     at the start of the program.
     """
+    global _registered
     signal.signal(signal.SIGCHLD, _zombieReaper)
+    _registered = True
 
 
 def unregisterSignalHandler():
@@ -68,4 +73,6 @@
     Stop cleaning PIDs. Should only be used for testing or other specialized
     use cases.
     """
+    global _registered
     signal.signal(signal.SIGCHLD, signal.SIG_DFL)
+    _registered = False
diff --git a/lib/vdsm/infra/zombiereaper/tests.py 
b/lib/vdsm/infra/zombiereaper/tests.py
index ee4fb02..6f1983f 100644
--- a/lib/vdsm/infra/zombiereaper/tests.py
+++ b/lib/vdsm/infra/zombiereaper/tests.py
@@ -25,10 +25,15 @@
 
 from unittest import TestCase
 
-zombiereaper.registerSignalHandler()
-
 
 class zombieReaperTests(TestCase):
+
+    def setUp(self):
+        zombiereaper.registerSignalHandler()
+
+    def tearDown(self):
+        zombiereaper.unregisterSignalHandler()
+
     def testProcessDiesAfterBeingTracked(self):
         p = CPopen(["sleep", "1"])
         zombiereaper.autoReapPID(p.pid)
@@ -47,3 +52,8 @@
 
         # Throws error because pid is not found or is not child
         self.assertRaises(OSError, os.waitpid, p.pid, os.WNOHANG)
+
+
+class RegisterTests(TestCase):
+    def testUnregistered(self):
+        self.assertRaises(RuntimeError, zombiereaper.autoReapPID, 12345)


-- 
To view, visit http://gerrit.ovirt.org/37260
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib3a275a3f022c7b686ce07693b7c0b147762b043
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to