[PATCH][python] dbus.connection: Release signals lock before calling _clean_up_signal_match().
Simon McVittie
simon.mcvittie at collabora.co.uk
Mon May 28 11:49:17 PDT 2007
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
This prevents a deadlock when a signal match that's tracking name owner
changes is removed, causing a match on NameOwnerChanged to be removed too.
(Debian bug#426412)
Also move more of the tree manipulation inside the lock, to be nice to
anyone attempting a port to implementations without a GIL (mainly IronPython),
and add a regression test for the above bug.
- ---
dbus/bus.py | 2 +-
dbus/connection.py | 45 +++++++++++++++++++++++++--------------------
test/test-client.py | 2 ++
test/test-signals.py | 34 ++++++++++++++++++++++++----------
4 files changed, 52 insertions(+), 31 deletions(-)
diff --git a/dbus/bus.py b/dbus/bus.py
index 9313289..04180a4 100644
- --- a/dbus/bus.py
+++ b/dbus/bus.py
@@ -147,7 +147,7 @@ class BusConnection(Connection):
return match
def _clean_up_signal_match(self, match):
- - # The signals lock must be held.
+ # The signals lock is no longer held here (it was in <= 0.81.0)
self.remove_match_string(str(match))
watch = self._signal_sender_matches.pop(match, None)
if watch is not None:
diff --git a/dbus/connection.py b/dbus/connection.py
index 1304058..78ed287 100644
- --- a/dbus/connection.py
+++ b/dbus/connection.py
@@ -245,9 +245,7 @@ class Connection(_Connection):
mapping member to list of SignalMatch objects."""
self._signals_lock = thread.allocate_lock()
- - """Lock used to protect signal data structures if doing two
- - removals at the same time (everything else is atomic, thanks to
- - the GIL)"""
+ """Lock used to protect signal data structures"""
self.add_message_filter(self.__class__._signal_func)
@@ -389,17 +387,18 @@ class Connection(_Connection):
match = SignalMatch(self, bus_name, path, dbus_interface,
signal_name, handler_function, **keywords)
- - by_interface = self._signal_recipients_by_object_path.setdefault(path,
- - {})
- - by_member = by_interface.setdefault(dbus_interface, {})
- - matches = by_member.setdefault(signal_name, [])
- - # make sure nobody is currently manipulating the list
self._signals_lock.acquire()
try:
+ by_interface = self._signal_recipients_by_object_path.setdefault(
+ path, {})
+ by_member = by_interface.setdefault(dbus_interface, {})
+ matches = by_member.setdefault(signal_name, [])
+
matches.append(match)
finally:
self._signals_lock.release()
+
return match
def _iter_easy_matches(self, path, dbus_interface, member):
@@ -450,18 +449,21 @@ class Connection(_Connection):
'positional parameters',
DeprecationWarning, stacklevel=2)
- - by_interface = self._signal_recipients_by_object_path.get(path, None)
- - if by_interface is None:
- - return
- - by_member = by_interface.get(dbus_interface, None)
- - if by_member is None:
- - return
- - matches = by_member.get(signal_name, None)
- - if matches is None:
- - return
+ new = []
+ deletions = []
self._signals_lock.acquire()
try:
- - new = []
+ by_interface = self._signal_recipients_by_object_path.get(path,
+ None)
+ if by_interface is None:
+ return
+ by_member = by_interface.get(dbus_interface, None)
+ if by_member is None:
+ return
+ matches = by_member.get(signal_name, None)
+ if matches is None:
+ return
+
for match in matches:
if (handler_or_match is match
or match.matches_removal_spec(bus_name,
@@ -470,15 +472,18 @@ class Connection(_Connection):
signal_name,
handler_or_match,
**keywords)):
- - self._clean_up_signal_match(match)
+ deletions.append(match)
else:
new.append(match)
by_member[signal_name] = new
finally:
self._signals_lock.release()
+ for match in deletions:
+ self._clean_up_signal_match(match)
+
def _clean_up_signal_match(self, match):
- - # Called with the signals lock held
+ # Now called without the signals lock held (it was held in <= 0.81.0)
pass
def _signal_func(self, message):
diff --git a/test/test-client.py b/test/test-client.py
index 04706df..49c4261 100755
- --- a/test/test-client.py
+++ b/test/test-client.py
@@ -68,6 +68,8 @@ class TestDBusBindings(unittest.TestCase):
def setUp(self):
self.bus = dbus.SessionBus()
self.remote_object = self.bus.get_object(NAME, OBJECT)
+ self.remote_object_follow = self.bus.get_object(NAME, OBJECT,
+ follow_name_owner_changes=True)
self.iface = dbus.Interface(self.remote_object, IFACE)
def testGObject(self):
diff --git a/test/test-signals.py b/test/test-signals.py
index f496de9..797f70c 100644
- --- a/test/test-signals.py
+++ b/test/test-signals.py
@@ -53,10 +53,12 @@ class TestSignals(unittest.TestCase):
logger.info('setUp()')
self.bus = dbus.SessionBus()
self.remote_object = self.bus.get_object("org.freedesktop.DBus.TestSuitePythonService", "/org/freedesktop/DBus/TestSuitePythonObject")
+ self.remote_object_follow = self.bus.get_object("org.freedesktop.DBus.TestSuitePythonService", "/org/freedesktop/DBus/TestSuitePythonObject", follow_name_owner_changes=True)
self.iface = dbus.Interface(self.remote_object, "org.freedesktop.DBus.TestSuiteInterface")
+ self.iface_follow = dbus.Interface(self.remote_object_follow, "org.freedesktop.DBus.TestSuiteInterface")
self.in_test = None
- - def signal_test_impl(self, name, test_removal=False):
+ def signal_test_impl(self, iface, name, test_removal=False):
self.in_test = name
# using append rather than assignment here to avoid scoping issues
result = []
@@ -79,11 +81,11 @@ class TestSignals(unittest.TestCase):
main_loop.quit()
logger.info('Testing %s', name)
- - match = self.iface.connect_to_signal('SignalOneString', _signal_handler,
- - sender_keyword='sender',
- - path_keyword='path')
+ match = iface.connect_to_signal('SignalOneString', _signal_handler,
+ sender_keyword='sender',
+ path_keyword='path')
logger.info('Waiting for signal...')
- - self.iface.EmitSignal('SignalOneString', 0)
+ iface.EmitSignal('SignalOneString', 0)
source_id = gobject.timeout_add(1000, _timeout_handler)
main_loop.run()
if not result:
@@ -95,7 +97,7 @@ class TestSignals(unittest.TestCase):
self.in_test = name + '+removed'
logger.info('Testing %s', name)
result = []
- - self.iface.EmitSignal('SignalOneString', 0)
+ iface.EmitSignal('SignalOneString', 0)
source_id = gobject.timeout_add(1000, _rm_timeout_handler)
main_loop.run()
if result:
@@ -103,16 +105,28 @@ class TestSignals(unittest.TestCase):
gobject.source_remove(source_id)
def testSignal(self):
- - self.signal_test_impl('Signal')
+ self.signal_test_impl(self.iface, 'Signal')
def testRemoval(self):
- - self.signal_test_impl('Removal', True)
+ self.signal_test_impl(self.iface, 'Removal', True)
def testSignalAgain(self):
- - self.signal_test_impl('SignalAgain')
+ self.signal_test_impl(self.iface, 'SignalAgain')
def testRemovalAgain(self):
- - self.signal_test_impl('RemovalAgain', True)
+ self.signal_test_impl(self.iface, 'RemovalAgain', True)
+
+ def testSignalF(self):
+ self.signal_test_impl(self.iface_follow, 'Signal')
+
+ def testRemovalF(self):
+ self.signal_test_impl(self.iface_follow, 'Removal', True)
+
+ def testSignalAgainF(self):
+ self.signal_test_impl(self.iface_follow, 'SignalAgain')
+
+ def testRemovalAgainF(self):
+ self.signal_test_impl(self.iface_follow, 'RemovalAgain', True)
if __name__ == '__main__':
main_loop = gobject.MainLoop()
- --
1.5.2-rc3.GIT
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: OpenPGP key: http://www.pseudorandom.co.uk/2003/contact/ or pgp.net
iD8DBQFGWyQtWSc8zVUw7HYRAiB+AKDMFyMNLP7UHR9LpKo+AOnj7QbryACfY7d0
lw69AC7JqjIjNNNQKE8pUWw=
=RQe1
-----END PGP SIGNATURE-----
More information about the dbus
mailing list