From 1b429e2ab672cda6fdf7648e789aaacb94f65d24 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Wed, 7 Apr 2010 14:41:16 +0200
Subject: [PATCH] Fix utils.WaitForFdCondition inner retry loop

Commit dfdc4060 added WaitForFdCondition which uses utils.Retry without
handling timeout exceptions. This breaks any nested retry loops.

This patch fixes the above function, and also changes utils.Retry to
detect and warn future similar cases. In addition, we add a few small
unittests for utils.Retry.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
 lib/daemon.py                 |  3 ++-
 lib/utils.py                  | 10 ++++++++--
 test/ganeti.utils_unittest.py | 23 +++++++++++++++++++++++
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/lib/daemon.py b/lib/daemon.py
index 0fba74db9..ff7708972 100644
--- a/lib/daemon.py
+++ b/lib/daemon.py
@@ -166,7 +166,8 @@ class AsyncUDPSocket(asyncore.dispatcher):
     @return: True if some data has been handled, False otherwise
 
     """
-    if utils.WaitForFdCondition(self, select.POLLIN, timeout) & select.POLLIN:
+    result = utils.WaitForFdCondition(self, select.POLLIN, timeout)
+    if result is not None and result & select.POLLIN:
       self.do_read()
       return True
     else:
diff --git a/lib/utils.py b/lib/utils.py
index c52e91ed2..c32ad206b 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -1579,8 +1579,11 @@ def WaitForFdCondition(fdobj, event, timeout):
   """
   if timeout is not None:
     retrywaiter = FdConditionWaiterHelper(timeout)
-    result = Retry(retrywaiter.Poll, RETRY_REMAINING_TIME, timeout,
-                   args=(fdobj, event), wait_fn=retrywaiter.UpdateTimeout)
+    try:
+      result = Retry(retrywaiter.Poll, RETRY_REMAINING_TIME, timeout,
+                     args=(fdobj, event), wait_fn=retrywaiter.UpdateTimeout)
+    except RetryTimeout:
+      result = None
   else:
     result = None
     while result is None:
@@ -2606,6 +2609,9 @@ def Retry(fn, delay, timeout, args=None, wait_fn=time.sleep,
       return fn(*args)
     except RetryAgain:
       pass
+    except RetryTimeout:
+      raise errors.ProgrammerError("Nested retry loop detected that didn't"
+                                   " handle RetryTimeout")
 
     remaining_time = end_time - _time_fn()
 
diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py
index 17d10fea6..e8f14c3bf 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -1535,5 +1535,28 @@ class TestMakedirs(unittest.TestCase):
     self.assert_(os.path.isdir(path))
 
 
+class TestRetry(testutils.GanetiTestCase):
+  @staticmethod
+  def _RaiseRetryAgain():
+    raise utils.RetryAgain()
+
+  def _WrongNestedLoop(self):
+    return utils.Retry(self._RaiseRetryAgain, 0.01, 0.02)
+
+  def testRaiseTimeout(self):
+    self.failUnlessRaises(utils.RetryTimeout, utils.Retry,
+                          self._RaiseRetryAgain, 0.01, 0.02)
+
+  def testComplete(self):
+    self.failUnlessEqual(utils.Retry(lambda: True, 0, 1), True)
+
+  def testNestedLoop(self):
+    try:
+      self.failUnlessRaises(errors.ProgrammerError, utils.Retry,
+                            self._WrongNestedLoop, 0, 1)
+    except utils.RetryTimeout:
+      self.fail("Didn't detect inner loop's exception")
+
+
 if __name__ == '__main__':
   testutils.GanetiTestProgram()
-- 
GitLab