Merge lp:~nafai/gwibber/gnomekeyring-fix into lp:gwibber

[Travis B. Hartwell]

This is a proposed fix for bug #554005.

Since accessing Gnome Keyring from threads causes a spike in CPU
usage, I have moved the access to gnome-keyring to the start of
gwibber-service, before threads are started. This section loads the
passwords from the keyring into a dictionary and then provides a
method for lookup. Then the calls to gnomekeyring are replaced with a
call to that method.

This patch handles the CPU pegging problem, but isn't a complete fix.
The following needs still to be done:

1) pitti proposed that the memory where this is stored should be
   mlock()'d. He suggested we use ctypes to wrap the calls to mlock
   and munlock. I've included stub functions, and pitti indicated he
   could help with wrapping this.

2) This implementation only reads the accounts and their passwords at
   the beginning of gwibber-service. The previous implementation
   caught changes to the passwords as well as the addition and removal
   of accounts by whenever actions are taken. Code still needs to be
   added to handle this. There are a couple of options that I can
   think of:

   a) Start and restart the gwibber service when these changes are
      made (in gwibber/accounts.py, I believe). I'm not sure if there
      are unforseen problems with this -- for example,
      gwibber-accounts can be run in a separate process, I believe.
      How does the main gwibber process handle loosing the gwibber
      service connection?

   b) Add a method to the com.Gwibber.Accounts dbus service for
      updating or adding account information (id and password). This
      method would then call a function we could add to
      gwibber.microblog.keyring to modify the passwords dictionary.
      The problem I see with this is that other developers who may
      look at the dbus services may assume this actually changes the
      passwords, not just the in-memory data structure.

   I imagine either of these options shouldn't be too hard to
   implement. I'm hoping that Ken or someone else can find a quick
   way to do this.

[Ken VanDine (ken-vandine)]

Good work, I'll look in more detail in the morning. Since gwibber-service is dbus activated, we could just make gwibber-accounts call the Quit method on the dispatcher and then before exiting itself start it back up via dbus. Before doing this, it should actually verify it was running.

[lp:~nafai/gwibber/gnomekeyring-fix]

712. By Travis B. Hartwell on 2010-04-08

Merge Ken's threading changes and other updates.

713. By Travis B. Hartwell on 2010-04-08

Move loading account passwords to the Dispatcher class and expose it
via DBus.

714. By Travis B. Hartwell on 2010-04-08

Access mlock/munlock via ctypes.

715. By Travis B. Hartwell on 2010-04-08

Call munlock properly

716. By Travis B. Hartwell on 2010-04-08

Clean-ups per pitti's suggestions.

=== modified file 'bin/gwibber-service'
--- bin/gwibber-service	2010-04-03 18:10:54 +0000
+++ bin/gwibber-service	2010-04-08 16:24:30 +0000
@@ -72,7 +72,7 @@
   log.logger.addHandler(console)
 
 ## Check to see if the database needs to be purged
-dispatcher.purge()
+#dispatcher.purge()
 
 log.logger.debug("Setting up monitors")
 connection_monitor = dispatcher.ConnectionMonitor()
@@ -83,6 +83,4 @@
 log.logger.debug("Monitors are up")
 
 dispatcher = dispatcher.Dispatcher(loop)
-dispatcher.daemon = True
-dispatcher.start()
 loop.run()

=== modified file 'gwibber/accounts.py'
--- gwibber/accounts.py	2010-04-03 17:20:33 +0000
+++ gwibber/accounts.py	2010-04-08 16:24:30 +0000
@@ -332,7 +332,6 @@
                 value = [getattr(widget.props, p) for p in ["text", "active", "color"] if widget and hasattr(widget.props, p)]
 
                 if value: 
-
                   if isinstance(value[0], gtk.gdk.Color):
                     self.account[config] = gtk.color_selection_palette_to_string(
                         gtk.color_selection_palette_from_string(value[0].to_string()))

=== modified file 'gwibber/microblog/dispatcher.py'
--- gwibber/microblog/dispatcher.py	2010-04-05 17:49:37 +0000
+++ gwibber/microblog/dispatcher.py	2010-04-08 16:24:30 +0000
@@ -1,13 +1,14 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
-import multiprocessing, threading, traceback, json, time
+import multiprocessing, traceback, json, time
 import gobject, dbus, dbus.service, mx.DateTime
 import twitter, identica, statusnet, flickr, facebook
-import qaiku, friendfeed, digg, gnomekeyring
+import qaiku, friendfeed, digg
 
 import urlshorter
 import util, util.couch
+import util.keyring
 from util import log
 from util import resources
 from util.couch import Monitor as CouchMonitor
@@ -58,11 +59,9 @@
 
     for key, val in account.items():
       if isinstance(val, str) and val.startswith(":KEYRING:"):
-        try:
-          value = gnomekeyring.find_items_sync(
-              gnomekeyring.ITEM_GENERIC_SECRET,
-              {"id": str("%s/%s" % (account["_id"], key))})[0].secret
-        except gnomekeyring.NoMatchError:
+        value = util.keyring.get_account_password(account["_id"])
+
+        if value is None:
           raise exceptions.GwibberProtocolError("keyring")
           return ("Failure", 0)
 
@@ -339,16 +338,14 @@
       expire_timeout = 5000
       n = util.notify(sender_name, message["text"], image, expire_timeout)
 
-class MapAsync(threading.Thread):
+class MapAsync:
   def __init__(self, func, iterable, cbsuccess, cbfailure, timeout=120):
-    threading.Thread.__init__(self)
     self.iterable = iterable
     self.callback = cbsuccess
     self.failure = cbfailure
     self.timeout = timeout
-    self.daemon = True
     self.func = func
-    self.start()
+    self.run()
 
   def run(self):
     try:
@@ -357,7 +354,7 @@
     except Exception as e:
       self.failure(e, traceback.format_exc())
 
-class Dispatcher(dbus.service.Object, threading.Thread):
+class Dispatcher(dbus.service.Object):
   """
   The Gwibber Dispatcher handles all the backend operations.
   """
@@ -367,13 +364,14 @@
     self.bus = dbus.SessionBus()
     bus_name = dbus.service.BusName("com.Gwibber.Service", bus=self.bus)
     dbus.service.Object.__init__(self, bus_name, self.__dbus_object_path__)
-    threading.Thread.__init__(self)
     
     self.collector = OperationCollector()
 
     self.refresh_count = 0
     self.mainloop = loop
 
+    self.RefreshCreds()
+    
     if autorefresh:
       self.refresh()
 
@@ -501,7 +499,20 @@
     """
     log.logger.info("Gwibber Service is being shutdown")
     self.mainloop.quit()
-  
+
+  @dbus.service.method("com.Gwibber.Service")
+  def RefreshCreds(self):
+    """
+    Reload accounts and credentials from Gnome Keyring
+    example:
+            import dbus
+            obj = dbus.SessionBus().get_object("com.Gwibber.Service", "/com/gwibber/Service")
+            service = dbus.Interface(obj, "com.Gwibber.Service")
+            service.RefreshCreds()
+    """
+    log.logger.info("Gwibber Service is reloading account credentials")
+    util.keyring.get_account_passwords()
+    
   def loading_complete(self, output):
     self.refresh_count += 1
     self.LoadingComplete()

=== added file 'gwibber/microblog/util/keyring.py'
--- gwibber/microblog/util/keyring.py	1970-01-01 00:00:00 +0000
+++ gwibber/microblog/util/keyring.py	2010-04-08 16:24:30 +0000
@@ -0,0 +1,49 @@
+from desktopcouch.records.server import CouchDatabase
+from const import *
+
+import atexit
+import ctypes
+import gnomekeyring
+
+passwords = {}
+
+def get_account_passwords():
+    global passwords
+
+    accounts = CouchDatabase(COUCH_DB_ACCOUNTS, create=True)
+    ids = [a['id'] for a in accounts.get_records().rows]
+
+    for id in ids:
+        account = dict(accounts.get_record(id).items())
+        for key, val in account.items():
+            if isinstance(val, str) and val.startswith(":KEYRING:"):
+                try:
+                    value = gnomekeyring.find_items_sync(
+                        gnomekeyring.ITEM_GENERIC_SECRET,
+                        {"id": str("%s/%s" % (account["_id"], key))})[0].secret
+                    mlock(value)
+                except gnomekeyring.NoMatchError:
+                    raise exceptions.GwibberProtocolError("keyring")
+
+                passwords[id] = value
+
+def get_account_password(accountid):
+    global passwords
+
+    return passwords.get(accountid, None)
+
+libc = ctypes.CDLL("libc.so.6")
+
+def mlock(var):
+    libc.mlock(var, len(var))
+
+def munlock(var):
+    libc.munlock(var, len(var))
+
+def munlock_passwords():
+    global passwords
+
+    for acctid in passwords.keys():
+        munlock(passwords[acctid])
+
+atexit.register(munlock_passwords)