Repository: qpid-python
Updated Branches:
  refs/heads/master 0c3c82b6b -> 7c968c831


QUID-7884: Python client should not raise on close() after stop.

The python client throws exceptions out of AMQP object methods (Connection, 
Session and Link objects) if the selector has been stopped, to prevent hanging 
(see QPID-7317 Deadlock on publish)

However to be robust to shut-down order, the close() method should not throw an 
exception in this case, but should be a no-op.


Project: http://git-wip-us.apache.org/repos/asf/qpid-python/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-python/commit/7c968c83
Tree: http://git-wip-us.apache.org/repos/asf/qpid-python/tree/7c968c83
Diff: http://git-wip-us.apache.org/repos/asf/qpid-python/diff/7c968c83

Branch: refs/heads/master
Commit: 7c968c8318f4c4a70fbe0ebbcdbe0a09d8cfbb3e
Parents: 0c3c82b
Author: Alan Conway <acon...@redhat.com>
Authored: Wed Aug 9 16:06:35 2017 -0400
Committer: Alan Conway <acon...@redhat.com>
Committed: Wed Aug 9 16:06:35 2017 -0400

----------------------------------------------------------------------
 qpid/selector.py                 | 15 +++++++++------
 qpid/tests/messaging/selector.py | 34 ++++++++++++++++++++++++++--------
 2 files changed, 35 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-python/blob/7c968c83/qpid/selector.py
----------------------------------------------------------------------
diff --git a/qpid/selector.py b/qpid/selector.py
index 56b137d..a67f0ae 100644
--- a/qpid/selector.py
+++ b/qpid/selector.py
@@ -205,12 +205,15 @@ class Selector:
     except Exception, e:
       log.error("error stopping qpid.messaging (%s)\n%s", self.exception, 
format_exc())
 
-# Disable an object so it raises exceptions on any use
+# Disable an object to avoid hangs due to forked mutex locks or a stopped 
selector thread
 import inspect
 def disable(obj, exception):
   assert(exception)
-  def log_raise(*args, **kwargs):
-    _check(exception, 1)
-  # Replace all methods with log_raise
-  for m in inspect.getmembers(obj, predicate=inspect.ismethod):
-    setattr(obj, m[0], log_raise)
+  # Replace methods to raise exception or be a no-op 
+  for m in inspect.getmembers(
+      obj, predicate=lambda m: inspect.ismethod(m) and not 
inspect.isbuiltin(m)):
+    name = m[0]
+    if name in ["close", "detach", "detach_all"]: # No-ops for these
+      setattr(obj, name, lambda *args, **kwargs: None)
+    else:                       # Raise exception for all others
+      setattr(obj, name, lambda *args, **kwargs: _check(exception, 1))

http://git-wip-us.apache.org/repos/asf/qpid-python/blob/7c968c83/qpid/tests/messaging/selector.py
----------------------------------------------------------------------
diff --git a/qpid/tests/messaging/selector.py b/qpid/tests/messaging/selector.py
index fc29607..6a40faf 100644
--- a/qpid/tests/messaging/selector.py
+++ b/qpid/tests/messaging/selector.py
@@ -20,9 +20,8 @@
 import sys, os
 from logging import getLogger
 from unittest import TestCase
-from qpid.selector import Selector
+from qpid.selector import Selector, SelectorStopped
 from qpid.messaging import *
-from qpid.messaging.exceptions import InternalError
 
 class SelectorTests(TestCase):
   """Make sure that using a connection after a selector stops raises and 
doesn't hang"""
@@ -43,16 +42,35 @@ class SelectorTests(TestCase):
   def test_use_after_stop(self):
     """Create endpoints, stop the selector, try to use them"""
     c = Connection.establish(self.broker)
+    cstr = str(c)
     ssn = c.session()
+    ssnrepr = repr(ssn)
     r = ssn.receiver("foo;{create:always,delete:always}")
+    rstr = str(r)
     s = ssn.sender("foo;{create:always,delete:always}")
+    srepr = str(s)
 
     Selector.DEFAULT.stop()
-    self.assertRaises(InternalError, c.session)
-    self.assertRaises(InternalError, ssn.sender, "foo")
-    self.assertRaises(InternalError, s.send, "foo")
-    self.assertRaises(InternalError, r.fetch)
-    self.assertRaises(InternalError, Connection.establish, self.broker)
+
+    # The following should be no-ops
+    c.close()
+    c.detach("foo")
+    ssn.close()
+    s.close()
+    r.close()
+
+    # str/repr should return the same result
+    self.assertEqual(cstr, str(c))
+    self.assertEqual(ssnrepr, repr(ssn))
+    self.assertEqual(rstr, str(r))
+    self.assertEqual(srepr, repr(s))
+
+    # Other functions should raise exceptions
+    self.assertRaises(SelectorStopped, c.session)
+    self.assertRaises(SelectorStopped, ssn.sender, "foo")
+    self.assertRaises(SelectorStopped, s.send, "foo")
+    self.assertRaises(SelectorStopped, r.fetch)
+    self.assertRaises(SelectorStopped, Connection.establish, self.broker)
 
   def test_use_after_fork(self):
     c = Connection.establish(self.broker)
@@ -64,7 +82,7 @@ class SelectorTests(TestCase):
       try:
         # Can establish new connections
         s = 
Connection.establish(self.broker).session().sender("child;{create:always}")
-        self.assertRaises(InternalError, c.session) # But can't use parent 
connection
+        self.assertRaises(SelectorStopped, c.session) # But can't use parent 
connection
         s.send("child")
         os._exit(0)
       except Exception, e:


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org

Reply via email to