Gwibber

Merge lp:~dobey/gwibber/gwibber-keyring-unthreaded into lp:gwibber

  1. gwibber-keyring-unthreaded
  2. Merge into trunk
Proposed by dobey
Status: Rejected
Rejected by: Ken VanDine
Proposed branch: lp:~dobey/gwibber/gwibber-keyring-unthreaded
Merge into: lp:gwibber
Diff against target: 50 lines (+3/-8)
2 files modified
bin/gwibber-service (+0/-2)
gwibber/microblog/dispatcher.py (+3/-6)
To merge this branch: bzr merge lp:~dobey/gwibber/gwibber-keyring-unthreaded
Reviewer Review Type Date Requested Status
Ken VanDine Disapprove
Review via email: mp+22704@code.launchpad.net

This proposal supersedes a proposal from 2010-03-31.

To post a comment you must log in.
Revision history for this message
dobey (dobey) wrote :

It appears that the association to gnome-keyring for this issue in gwibber is only somewhat incidental. The Dispatcher class in gwibber was mixing Threads and the gobject main loop with out mutex locking or any means of protecting against thread locks. There's really no good reason to derive the Dispatcher class from threading.Thread anyway, so I've just removed it instead.

If you want to re-introduce the use of threading.Thread here though, you should implement proper entry/exit to the main loop for calling gobject things (like idle_add/timeout_add*). And you should probably do it via a more generic class which all your dbus.Object implementations derive from (so that they are all separate threads).

Revision history for this message
Ken VanDine (ken-vandine) wrote :

Partially used, thanks!

review: Disapprove

Unmerged revisions

705. By dobey

Remove the use of Thread from the Dispatcher

704. By dobey

Use the with statement and gtk main loop lock to call gnomekeyring with

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/gwibber-service'
2--- bin/gwibber-service 2010-03-11 20:05:07 +0000
3+++ bin/gwibber-service 2010-04-02 15:48:19 +0000
4@@ -80,6 +80,4 @@
5 log.logger.debug("Monitors are up")
6
7 dispatcher = dispatcher.Dispatcher(loop)
8-dispatcher.daemon = True
9-dispatcher.start()
10 loop.run()
11
12=== modified file 'gwibber/microblog/dispatcher.py'
13--- gwibber/microblog/dispatcher.py 2010-03-31 16:51:12 +0000
14+++ gwibber/microblog/dispatcher.py 2010-04-02 15:48:19 +0000
15@@ -24,8 +24,6 @@
16 except:
17 indicate = None
18
19-gobject.threads_init()
20-
21 log.logger.name = "Gwibber Dispatcher"
22
23 PROTOCOLS = {
24@@ -54,8 +52,8 @@
25 if isinstance(val, str) and val.startswith(":KEYRING:"):
26 try:
27 value = gnomekeyring.find_items_sync(
28- gnomekeyring.ITEM_GENERIC_SECRET,
29- {"id": str("%s/%s" % (account["_id"], key))})[0].secret
30+ gnomekeyring.ITEM_GENERIC_SECRET,
31+ {"id": str("%s/%s" % (account["_id"], key))})[0].secret
32 except gnomekeyring.NoMatchError:
33 raise exceptions.GwibberProtocolError("keyring")
34 return ("Failure", 0)
35@@ -345,7 +343,7 @@
36 except Exception as e:
37 self.failure(e, traceback.format_exc())
38
39-class Dispatcher(dbus.service.Object, threading.Thread):
40+class Dispatcher(dbus.service.Object):
41 """
42 The Gwibber Dispatcher handles all the backend operations.
43 """
44@@ -355,7 +353,6 @@
45 self.bus = dbus.SessionBus()
46 bus_name = dbus.service.BusName("com.Gwibber.Service", bus=self.bus)
47 dbus.service.Object.__init__(self, bus_name, self.__dbus_object_path__)
48- threading.Thread.__init__(self)
49
50 self.collector = OperationCollector()
51

Subscribers

People subscribed via source and target branches