Merge lp:~cmiller/desktopcouch/segv-ya-rly into lp:desktopcouch

Proposed by Chad Miller
Status: Merged
Approved by: Tim Cole
Approved revision: 143
Merged at revision: not available
Proposed branch: lp:~cmiller/desktopcouch/segv-ya-rly
Merge into: lp:desktopcouch
Diff against target: 59 lines (+15/-4)
2 files modified
desktopcouch/replication_services/example.py (+3/-1)
desktopcouch/replication_services/ubuntuone.py (+12/-3)
To merge this branch: bzr merge lp:~cmiller/desktopcouch/segv-ya-rly
Reviewer Review Type Date Requested Status
Tim Cole (community) Approve
Guillermo Gonzalez Approve
Review via email: mp+22417@code.launchpad.net

Commit message

Since replication takes a long time (and we are not clever enough yet to replicate asynchronously), all replication is pushed into a subthread so that we don't block DBus server calls on getPort and such.

However, for some GUI libraries, GUI functions are not allowed to be outside the main execution thread. Violating that causes undefined behavior.

We need access to the keyring data in replication, and 'gnomekeyring' may try to display an alert to the user, using GUI functions. Doing so from inside the replication thread, outside the main thread, can cause SEGV crashes in deep and mysterious parts of Python and libraries it uses.

Now, add a wrapper for those functions that can call GUI functions. First test what environment we're in and if in a subthread, ask the twisted reactor to run the target function in the main execution thread and return the values down to the subthread.

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

looks good

review: Approve
Revision history for this message
Tim Cole (tcole) wrote :

Awesome. I'm glad we found and addressed the real problem.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'desktopcouch/replication_services/example.py'
2--- desktopcouch/replication_services/example.py 2009-09-28 15:13:33 +0000
3+++ desktopcouch/replication_services/example.py 2010-03-30 00:11:19 +0000
4@@ -13,7 +13,9 @@
5
6 # Required
7 def oauth_data():
8- """OAuth information needed to replicate to a server."""
9+ """OAuth information needed to replicate to a server. This may be
10+ called from a subthread, so be sure not to violate any execution
11+ idioms, like updating the GUI from a non-main thread."""
12 return dict(consumer_key="", consumer_secret="", oauth_token="",
13 oauth_token_secret="")
14 # or to symbolize failure
15
16=== modified file 'desktopcouch/replication_services/ubuntuone.py'
17--- desktopcouch/replication_services/ubuntuone.py 2010-03-10 22:27:28 +0000
18+++ desktopcouch/replication_services/ubuntuone.py 2010-03-30 00:11:19 +0000
19@@ -17,6 +17,15 @@
20 oauth_consumer_key = "ubuntuone"
21 oauth_consumer_secret = "hammertime"
22
23+def in_main_thread(function, *args, **kwargs):
24+ from twisted.python.threadable import isInIOThread
25+ if isInIOThread(): # We may not be in main thread right now.
26+ from twisted.internet.threads import blockingCallFromThread
27+ from twisted.internet import reactor
28+ return blockingCallFromThread(reactor, function, *args, **kwargs)
29+ else: # If in main, just do it.
30+ return function(*args, **kwargs)
31+
32 def is_active():
33 """Can we deliver information?"""
34 return get_oauth_data() is not None
35@@ -29,7 +38,7 @@
36 return oauth_data
37
38 try:
39- matches = gnomekeyring.find_items_sync(
40+ matches = in_main_thread(gnomekeyring.find_items_sync,
41 gnomekeyring.ITEM_GENERIC_SECRET,
42 {'ubuntuone-realm': "https://ubuntuone.com",
43 'oauth-consumer-key': oauth_consumer_key})
44@@ -58,13 +67,13 @@
45 def get_oauth_token(consumer):
46 """Get the token from the keyring"""
47 try:
48- items = gnomekeyring.find_items_sync(
49+ items = in_main_thread(gnomekeyring.find_items_sync,
50 gnomekeyring.ITEM_GENERIC_SECRET,
51 {'ubuntuone-realm': "https://one.ubuntu.com",
52 'oauth-consumer-key': consumer.key})
53 except gnomekeyring.NoMatchError:
54 logging.info("No o.u.c key. Maybe there's uo.c key?")
55- items = gnomekeyring.find_items_sync(
56+ items = in_main_thread(gnomekeyring.find_items_sync,
57 gnomekeyring.ITEM_GENERIC_SECRET,
58 {'ubuntuone-realm': "https://ubuntuone.com",
59 'oauth-consumer-key': consumer.key})

Subscribers

People subscribed via source and target branches