[patch] [python] Finish implementing deferred methods (take 2)

Simon McVittie simon.mcvittie at collabora.co.uk
Wed Jan 17 05:22:36 PST 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

As usual, pullable from
http://people.freedesktop.org/~smcv/git/dbus-python/.git

tsuraan: I'll have a look at your test case and see if I can understand
what's going on there.

- From 43c48b9c7fdbb5741daa024df9e7a319d1993dac Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Wed, 17 Jan 2007 13:06:33 +0000
Subject: [PATCH] dbus/proxies.py: Finish implementing deferred methods so they can be async.

* Queue up async methods and execute them when introspection finishes, instead
  of blocking on the introspection operation (heavily based on patch by J5)
* Rename DeferedMethod (sic) to spell Deferred correctly, and rename to
  _DeferredMethod (also _ProxyMethod) since these classes are not public API
* Make it safe to keep a reference to a DeferredMethod and call it with
  differing arguments:
    meth = proxy.DoStuff
    meth(1, reply_handler=on_reply, error_handler=on_error)
    meth(2, reply_handler=on_reply, error_handler=on_error)
* Make it safe to keep references to DeferredMethod even after introspection
  has finished - if called after introspection finishes, silently do an
  immediate call
* Add some locking to avoid subtle failures if one thread appends
  to the pending introspect queue at the same time another thread gets
  introspection results back - ProxyObject and friends should now be
  threadsafe (I think)
- ---
 dbus/proxies.py |  145 +++++++++++++++++++++++++++++++++----------------------
 1 files changed, 87 insertions(+), 58 deletions(-)

diff --git a/dbus/proxies.py b/dbus/proxies.py
index 66dc9a9..85ac3c2 100644
- --- a/dbus/proxies.py
+++ b/dbus/proxies.py
@@ -1,7 +1,7 @@
- -# Copyright (C) 2003, 2004, 2005, 2006 Red Hat Inc. <http://www.redhat.com/>
+# Copyright (C) 2003, 2004, 2005, 2006, 2007 Red Hat Inc. <http://www.redhat.com/>
 # Copyright (C) 2003 David Zeuthen
 # Copyright (C) 2004 Rob Taylor
- -# Copyright (C) 2005, 2006 Collabora Ltd. <http://www.collabora.co.uk/>
+# Copyright (C) 2005, 2006, 2007 Collabora Ltd. <http://www.collabora.co.uk/>
 #
 # Licensed under the Academic Free License version 2.1
 #
@@ -24,6 +24,11 @@
 import sys
 import logging
 
+try:
+    from threading import RLock
+except ImportError:
+    from dummy_threading import RLock
+
 import _dbus_bindings
 from dbus._expat_introspect_parser import process_introspection_data
 from dbus.exceptions import MissingReplyHandlerException, MissingErrorHandlerException, IntrospectionParserException, DBusException
@@ -60,31 +65,30 @@ class _ReplyHandler(object):
                                         % message))
 
 
- -class DeferedMethod:
- -    """A DeferedMethod
- -    
- -    This is returned instead of ProxyMethod when we are defering DBus calls
- -    while waiting for introspection data to be returned
+class _DeferredMethod:
+    """A proxy method which will only get called once we have its
+    introspection reply.
     """
- -    def __init__(self, proxy_method):
+    def __init__(self, proxy_method, append, block):
         self._proxy_method = proxy_method
- -        self._method_name  = proxy_method._method_name
- -    
+        # the test suite relies on the existence of this property
+        self._method_name = proxy_method._method_name
+        self._append = append
+        self._block = block
+
     def __call__(self, *args, **keywords):
- -        reply_handler = None
         if keywords.has_key('reply_handler'):
- -            reply_handler = keywords['reply_handler']
- -
- -        #block for now even on async
- -        # FIXME: put ret in async queue in future if we have a reply handler
+            # defer the async call til introspection finishes
+            self._append(self._proxy_method, args, keywords)
+            return None
+        else:
+            # we're being synchronous, so block
+            self._block()
+            return self._proxy_method(*args, **keywords)
 
- -        self._proxy_method._proxy._pending_introspect.block()
- -        ret = self._proxy_method (*args, **keywords)
- -        
- -        return ret
 
- -class ProxyMethod:
- -    """A proxy Method.
+class _ProxyMethod:
+    """A proxy method.
 
     Typically a member of a ProxyObject. Calls to the
     method produce messages that travel over the Bus and are routed
@@ -95,6 +99,7 @@ class ProxyMethod:
         self._connection     = connection
         self._named_service  = named_service
         self._object_path    = object_path
+        # the test suite relies on the existence of this property
         self._method_name    = method_name
         self._dbus_interface = iface
 
@@ -179,8 +184,8 @@ class ProxyObject:
     A ProxyObject is provided by the Bus. ProxyObjects
     have member functions, and can be called like normal Python objects.
     """
- -    ProxyMethodClass = ProxyMethod
- -    DeferedMethodClass = DeferedMethod
+    ProxyMethodClass = _ProxyMethod
+    DeferredMethodClass = _DeferredMethod
 
     INTROSPECT_STATE_DONT_INTROSPECT = 0
     INTROSPECT_STATE_INTROSPECT_IN_PROGRESS = 1
@@ -219,7 +224,7 @@ class ProxyObject:
                 self._named_service = bus_object.GetNameOwner(named_service,
                         dbus_interface=BUS_DAEMON_IFACE)
             except DBusException, e:
- -                # FIXME: detect whether it's NameHasNoOwner
+                # FIXME: detect whether it's NameHasNoOwner, but properly
                 #if not str(e).startswith('org.freedesktop.DBus.Error.NameHasNoOwner:'):
                 #    raise
                 # it might not exist: try to start it
@@ -235,6 +240,10 @@ class ProxyObject:
         #dictionary mapping method names to their input signatures
         self._introspect_method_map = {}
 
+        # must be a recursive lock because block() is called while locked,
+        # and calls the callback which re-takes the lock
+        self._introspect_lock = RLock()
+
         if not introspect:
             self._introspect_state = self.INTROSPECT_STATE_DONT_INTROSPECT
         else:
@@ -317,43 +326,58 @@ class ProxyObject:
         return result
     
     def _introspect_execute_queue(self):
- -        for call in self._pending_introspect_queue:
- -            (member, iface, args, keywords) = call
- -
- -            introspect_sig = None
- -
- -            tmp_iface = ''
- -            if iface:
- -                tmp_iface = iface + '.'
- -                    
- -            key = tmp_iface + '.' + member
- -            if self._introspect_method_map.has_key (key):
- -                introspect_sig = self._introspect_method_map[key]
- -
- -            
- -            call_object = self.ProxyMethodClass(self._bus.get_connection(),
- -                                                self._named_service,
- -                                                self.__dbus_object_path__,
- -                                                iface,
- -                                                member,
- -                                                introspect_sig)
- -                                                                       
- -            call_object(args, keywords)
+        # FIXME: potential to flood the bus
+        # We should make sure mainloops all have idle handlers
+        # and do one message per idle
+        for (proxy_method, args, keywords) in self._pending_introspect_queue:
+            proxy_method(*args, **keywords)
 
     def _introspect_reply_handler(self, data):
+        self._introspect_lock.acquire()
         try:
- -            self._introspect_method_map = process_introspection_data(data)
- -        except IntrospectionParserException, e:
- -            self._introspect_error_handler(e)
- -            return
- -        
- -        self._introspect_state = self.INTROSPECT_STATE_INTROSPECT_DONE
- -        #self._introspect_execute_queue()
+            try:
+                self._introspect_method_map = process_introspection_data(data)
+            except IntrospectionParserException, e:
+                self._introspect_error_handler(e)
+                return
+
+            self._introspect_state = self.INTROSPECT_STATE_INTROSPECT_DONE
+            self._pending_introspect = None
+            self._introspect_execute_queue()
+        finally:
+            self._introspect_lock.release()
 
     def _introspect_error_handler(self, error):
- -        self._introspect_state = self.INTROSPECT_STATE_DONT_INTROSPECT
- -        self._introspect_execute_queue()
- -        sys.stderr.write("Introspect error: " + str(error) + "\n")
+        self._introspect_lock.acquire()
+        try:
+            self._introspect_state = self.INTROSPECT_STATE_DONT_INTROSPECT
+            self._pending_introspect = None
+            self._introspect_execute_queue()
+            sys.stderr.write("Introspect error: " + str(error) + "\n")
+        finally:
+            self._introspect_lock.release()
+
+    def _introspect_block(self):
+        self._introspect_lock.acquire()
+        try:
+            if self._pending_introspect is not None:
+                self._pending_introspect.block()
+            # else someone still has a _DeferredMethod from before we
+            # finished introspection: no need to do anything special any more
+        finally:
+            self._introspect_lock.release()
+
+    def _introspect_add_to_queue(self, callback, args, kwargs):
+        self._introspect_lock.acquire()
+        try:
+            if self._introspect_state == self.INTROSPECT_STATE_INTROSPECT_IN_PROGRESS:
+                self._pending_introspect_queue.append((callback, args, kwargs))
+            else:
+                # someone still has a _DeferredMethod from before we
+                # finished introspection
+                callback(*args, **kwargs)
+        finally:
+            self._introspect_lock.release()
 
     def __getattr__(self, member, dbus_interface=None):
         if member == '__call__':
@@ -390,9 +414,14 @@ class ProxyObject:
                                     self._named_service,
                                     self.__dbus_object_path__, member,
                                     dbus_interface)
- -    
+
+        # this can be done without taking the lock - the worst that can
+        # happen is that we accidentally return a _DeferredMethod just after
+        # finishing introspection, in which case _introspect_add_to_queue and
+        # _introspect_block will do the right thing anyway
         if self._introspect_state == self.INTROSPECT_STATE_INTROSPECT_IN_PROGRESS:
- -            ret = self.DeferedMethodClass(ret)
+            ret = self.DeferredMethodClass(ret, self._introspect_add_to_queue,
+                                           self._introspect_block)
 
         return ret
 
- -- 
1.4.4.4

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: OpenPGP key: http://www.pseudorandom.co.uk/2003/contact/ or pgp.net

iD8DBQFFriMcWSc8zVUw7HYRAk0+AJ9xvFbScflI7HZK3GuxVs4GGnoAPACdFbUI
2DSz2wheXJjVAsD7Ndwsd9I=
=IFtK
-----END PGP SIGNATURE-----


More information about the dbus mailing list