Apr 13 12

The windows tests are dirty!!

by mandel

The last few days I have taken a considerable time to remove all the errors from the Ubuntu One tests on Windows. The tests were leaving tcp connections in the twisted reactor that on some Windows systems will result on a DirtyReactorError which is a PITA. Due to a refactor the pattern I described here was remove (ouch!) and I had to re-add it. In this case, instead of doing the pattern everywhere is needed I created some helper classes that will ensure that the clients and servers are correctly cleaned up after you use them.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200

from twisted.internet import defer, protocol, reactor
from twisted.spread import pb
 
from ubuntuone.devtools.testcases import BaseTestCase
 
# no init method +  twisted common warnings
# pylint: disable=W0232, C0103, E1101
 
 
def server_protocol_factory(cls):
    """Factory to create tidy protocols."""
 
    if cls is None:
        cls = protocol.Protocol
 
    class ServerSaveProtocol(cls):
        """A tidy protocol."""
 
        def connectionLost(self, *args):
            """Lost the connection."""
            cls.connectionLost(self, *args)
            # lets tell everyone
            # pylint: disable=W0212
            if (self.factory._disconnecting
                    and self.factory.testserver_on_connection_lost is not None
                    and not self.factory.testserver_on_connection_lost.called):
                self.factory.testserver_on_connection_lost.callback(self)
            # pylint: enable=W0212
 
    return ServerSaveProtocol
 
 
def client_protocol_factory(cls):
    """Factory to create tidy protocols."""
 
    if cls is None:
        cls = protocol.Protocol
 
    class ClientSaveProtocol(cls):
        """A tidy protocol."""
 
        def connectionMade(self):
            """Connection made."""
            if (self.factory.testserver_on_connection_made is not None
                    and not self.factory.testserver_on_connection_made.called):
                self.factory.testserver_on_connection_made.callback(self)
            cls.connectionMade(self)
 
        def connectionLost(self, *a):
            """Connection list."""
            # pylint: disable=W0212
            if (self.factory._disconnecting
                    and self.factory.testserver_on_connection_lost is not None
                    and not self.factory.testserver_on_connection_lost.called):
                self.factory.testserver_on_connection_lost.callback(self)
            # pylint: enable=W0212
            cls.connectionLost(self, *a)
 
    return ClientSaveProtocol
 
 
class TidyTCPServer(object):
    """Ensure that twisted services are correctly managed in tests.
 
    Closing a twisted service is a complicated matter. In order to do so you
    have to ensure that three different deferreds are fired:
 
        1. The server must stop listening.
        2. The client connection must disconnect.
        3. The server connection must disconnect.
 
    This class allows to create a service and a client that will ensure that
    the reactor is left clean by following the pattern described at
    http://mumak.net/stuff/twisted-disconnect.html
    """
 
    def __init__(self):
        """Create a new instance."""
        self.listener = None
        self.server_factory = None
 
        self.connector = None
        self.client_factory = None
 
    def listen_server(self, server_class, *args, **kwargs):
        """Start a server in a random port."""
        # pylint: disable=W0621
        from twisted.internet import reactor
        # pylint: enable=W0621
        self.server_factory = server_class(*args, **kwargs)
        self.server_factory._disconnecting = False
        self.server_factory.testserver_on_connection_lost = defer.Deferred()
        self.server_factory.protocol = server_protocol_factory(
                                                 self.server_factory.protocol)
        self.listener = reactor.listenTCP(0, self.server_factory)
        return self.server_factory
 
    def connect_client(self, client_class, *args, **kwargs):
        """Conect a client to a given service."""
 
        if self.server_factory is None:
            raise ValueError('Server Factory was not provided.')
        if self.listener is None:
            raise ValueError('%s has not started listening.',
                                                          self.server_factory)
 
        self.client_factory = client_class(*args, **kwargs)
        self.client_factory._disconnecting = False
        self.client_factory.protocol = client_protocol_factory(
                                                 self.client_factory.protocol)
        self.client_factory.testserver_on_connection_made = defer.Deferred()
        self.client_factory.testserver_on_connection_lost = defer.Deferred()
        self.connector = reactor.connectTCP('localhost',
                                        self.listener.getHost().port,
                                        self.client_factory)
        return self.client_factory
 
    def clean_up(self):
        """Action to be performed for clean up."""
        if self.server_factory is None or self.listener is None:
            # nothing to clean
            return defer.succeed(None)
 
        if self.listener and self.connector:
            # clean client and server
            self.server_factory._disconnecting = True
            self.client_factory._disconnecting = True
            d = defer.maybeDeferred(self.listener.stopListening)
            self.connector.disconnect()
            return defer.gatherResults([d,
                self.client_factory.testserver_on_connection_lost,
                self.server_factory.testserver_on_connection_lost])
        if self.listener:
            # just clean the server since there is no client
            self.server_factory._disconnecting = True
            return defer.maybeDeferred(self.listener.stopListening)
 
 
class TCPServerTestCase(BaseTestCase):
    """Test that uses a single twisted service."""
 
    @defer.inlineCallbacks
    def setUp(self):
        """Set the diff tests."""
        yield super(TCPServerTestCase, self).setUp()
        self.service_runner = TidyTCPServer()
 
        self.server_factory = None
        self.client_factory = None
        self.server_disconnected = None
        self.client_connected = None
        self.client_disconnected = None
        self.listener = None
        self.connector = None
        self.addCleanup(self.tear_down_server_client)
 
    def listen_server(self, server_class, *args, **kwargs):
        """Listen a server.
 
        The method takes the server class and the arguments that should be
        passed to the server constructor.
        """
        self.server_factory = self.service_runner.listen_server(server_class,
                *args, **kwargs)
        self.server_disconnected = \
                self.server_factory.testserver_on_connection_lost
        self.listener = self.service_runner.listener
 
    def connect_client(self, client_class, *args, **kwargs):
        """Connect the client.
 
        The method takes the client factory  class and the arguments that
        should be passed to the client constructor.
        """
        self.client_factory = self.service_runner.connect_client(client_class,
                                                              *args, **kwargs)
        self.client_connected = \
                self.client_factory.testserver_on_connection_made
        self.client_disconnected = \
                self.client_factory.testserver_on_connection_lost
        self.connector = self.service_runner.connector
 
    def tear_down_server_client(self):
        """Clean the server and client."""
        if self.server_factory and self.client_factory:
            return self.service_runner.clean_up()
 
 
class PbServiceTestCase(TCPServerTestCase):
    """Test a pb service."""
 
    def listen_server(self, *args, **kwargs):
        """Listen a pb server."""
        super(PbServiceTestCase, self).listen_server(pb.PBServerFactory,
                                                              *args, **kwargs)
 
    def connect_client(self, *args, **kwargs):
        """Connect a pb client."""
        super(PbServiceTestCase, self).connect_client(pb.PBClientFactory,
                                                              *args, **kwargs)

The above classes can be used in the following way:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108

from twisted.internet import defer, protocol
from twisted.spread import pb
from twisted.trial.unittest import TestCase
 
from ubuntuone.devtools.testcases.txtcpserver import (
    client_protocol_factory,
    server_protocol_factory,
    PbServiceTestCase,
    TidyTCPServer,
    TCPServerTestCase,
)
 
# no init
# pylint: disable=W0232
 
 
class Adder(pb.Referenceable):
    """A remote adder."""
 
    def remote_add(self, first, second):
        """Remote adding numbers."""
        return first + second
 
 
class Calculator(pb.Root):
    """A calculator ran somewhere on the net."""
 
    def __init__(self, adder):
        """Create a new instance."""
         # pb.Root has no __init__
        self.adder = adder
 
    def remote_get_adder(self):
        """Get the remote added."""
        return self.adder
 
    def remote_check_adder(self, other_adder):
        """Check if the are the same."""
        return self.adder == other_adder
 
 
class Echoer(pb.Root):
    """An echoer that repeats what we say."""
 
    def remote_say(self, sentence):
        """Echo what we want to say."""
        return 'Echoer: %s' % sentence
# pylint: enable=W0232
 
class MultipleSercicesTestCase(TestCase):
    """Ensure that several services can be ran."""
 
    timeout = 2
 
    @defer.inlineCallbacks
    def setUp(self):
        """Set the diff tests."""
        yield super(MultipleSercicesTestCase, self).setUp()
        self.first_tcp_server = TidyTCPServer()
        self.second_tcp_server = TidyTCPServer()
        self.adder = Adder()
        self.calculator = Calculator(self.adder)
        self.echoer = Echoer()
 
    @defer.inlineCallbacks
    def test_single_service(self):
        """Test setting a single service."""
        first_number = 1
        second_number = 2
        self.first_tcp_server.listen_server(pb.PBServerFactory,
                                                              self.calculator)
        self.addCleanup(self.first_tcp_server.clean_up)
        calculator_c = self.first_tcp_server.connect_client(pb.PBClientFactory)
        # ensure we do have connected
        yield calculator_c.testserver_on_connection_made
        calculator = yield calculator_c.getRootObject()
        adder = yield calculator.callRemote('get_adder')
        result = yield adder.callRemote('add', first_number, second_number)
        self.assertEqual(first_number + second_number, result)
 
    @defer.inlineCallbacks
    def test_multiple_services(self):
        """Test setting multiple services."""
        first_number = 1
        second_number = 2
        # first service
        self.first_tcp_server.listen_server(pb.PBServerFactory,
                                                              self.calculator)
        self.addCleanup(self.first_tcp_server.clean_up)
 
        # second service
        self.second_tcp_server.listen_server(pb.PBServerFactory, self.echoer)
        self.addCleanup(self.second_tcp_server.clean_up)
 
        # connect the diff clients
        calculator_c = self.first_tcp_server.connect_client(pb.PBClientFactory)
        echoer_c = self.second_tcp_server.connect_client(pb.PBClientFactory)
        # ensure we do have connected
        yield calculator_c.testserver_on_connection_made
        yield echoer_c.testserver_on_connection_made
 
        calculator = yield calculator_c.getRootObject()
        adder = yield calculator.callRemote('get_adder')
        result = yield adder.callRemote('add', first_number, second_number)
        self.assertEqual(first_number + second_number, result)
        echoer = yield echoer_c.getRootObject()
        echo = yield echoer.callRemote('say', 'hello')
        self.assertEqual(self.echoer.remote_say('hello'), echo)

On top of this helper classes I added a twisted site that would follow a similar pattern:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94

"""A tx based web server."""
 
from twisted.internet import defer, reactor, ssl
from twisted.protocols.policies import WrappingFactory
from twisted.web import server
 
from ubuntuone.devtools.testcases.txtcpserver import server_protocol_factory
 
# no init method +  twisted common warnings
# pylint: disable=W0232, C0103, E1101
 
 
class WebServer(object):
    """Webserver used to perform requests in tests."""
 
    def __init__(self, root_resource, ssl_settings=None):
        """Create and start the instance.
 
        The ssl_settings parameter contains a dictionary with the key and cert
        that will be used to perform ssl connections. The root_resource
        contains the resource with all its childre.
        """
        self.root = root_resource
        self.ssl_settings = ssl_settings
        self.port = None
        # use an http.HTTPFactory that was modified to ensure that we have
        # clean close connections
        self.site = server.Site(self.root, timeout=None)
        self.wrapper = WrappingFactory(self.site)
        self.wrapper.testserver_on_connection_lost = defer.Deferred()
        self.wrapper.protocol = server_protocol_factory(self.wrapper.protocol)
        self.wrapper._disconnecting = False
 
    def _listen(self, site, ssl_settings=None):
        """Listen a port to allow the tests."""
        if ssl_settings is None:
            return reactor.listenTCP(0, site)
        else:
            ssl_context = ssl.DefaultOpenSSLContextFactory(
                                    ssl_settings['key'], ssl_settings['cert'])
            return reactor.listenSSL(0, site, ssl_context)
 
    def get_iri(self):
        """Build the iri for this mock server."""
        url = u"http://127.0.0.1:%d/"
        return url % self.get_port()
 
    def get_uri(self):
        """Build the uri for this mock server."""
        url = "http://127.0.0.1:%d/"
        return url % self.get_port()
 
    def get_ssl_iri(self):
        """Build the ssl iri for this mock server."""
        if self.ssl_settings:
            url = u"https://127.0.0.1:%d/"
            return url % self.get_ssl_port()
 
    def get_ssl_uri(self):
        """Build the ssl iri for this mock server."""
        if self.ssl_settings:
            url = "https://127.0.0.1:%d/"
            return url % self.get_ssl_port()
 
    def get_port(self):
        """Return the port where we are listening."""
        return self.port.getHost().port
 
    def get_ssl_port(self):
        """Return the ssl port where we are listening."""
        # pylint: disable=W0212
        if self.ssl_settings:
            return self.port.getHost().port
        # pylint: enable=W0212
 
    def start(self):
        """Start the service."""
        self.port = self._listen(self.wrapper, self.ssl_settings)
 
    def stop(self):
        """Shut it down."""
        if self.port:
            self.wrapper._disconnecting = True
            connected = self.wrapper.protocols.keys()
            if connected:
                for con in connected:
                    con.transport.loseConnection()
            else:
                self.wrapper.testserver_on_connection_lost = \
                                                            defer.succeed(None)
            d = defer.maybeDeferred(self.port.stopListening)
            return defer.gatherResults([d,
                                  self.wrapper.testserver_on_connection_lost])
        return defer.succeed(None)

When using this webserver you have to be careful because we do not longer pay attention to the client protocols, if you do not use twisted to access it you have no problems (libsoup, qtnetwork etc..) but if, for example, you use the HTTPClientFactory you have to do something similar to this in your TestCase setUp:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50

class TxWebClientTestCase(WebClientTestCase):
    """Test case for txweb."""
 
    webclient_factory = txweb.WebClient
 
    @defer.inlineCallbacks
    def setUp(self):
        """Set the diff tests."""
        # delay import, otherwise a default reactor gets installed
        from twisted.web import client
        self.factory = client.HTTPClientFactory
        # set the factory to be used
        # Hook the server's buildProtocol to make the protocol instance
        # accessible to tests and ensure that the reactor is clean!
        build_protocol = self.factory.buildProtocol
        self.serverProtocol = None
 
        def remember_protocol_instance(my_self, addr):
            """Remember the protocol used in the test."""
            protocol = build_protocol(my_self, addr)
            self.serverProtocol = protocol
            on_connection_lost = defer.Deferred()
            connection_lost = protocol.connectionLost
 
            def defer_connection_lost(protocol, *a):
                """Lost connection."""
                if not on_connection_lost.called:
                    on_connection_lost.callback(None)
                connection_lost(protocol, *a)
 
            self.patch(protocol, 'connectionLost', defer_connection_lost)
 
            def cleanup():
                """Clean the connection."""
                if self.serverProtocol.transport is not None:
                    self.serverProtocol.transport.loseConnection()
                return on_connection_lost
 
            self.addCleanup(cleanup)
            return protocol
 
        self.factory.buildProtocol = remember_protocol_instance
        self.addCleanup(self.set_build_protocol, build_protocol)
        txweb.WebClient.client_factory = self.factory
 
        yield super(TxWebClientTestCase, self).setUp()
 
    def set_build_protocol(self, method):
        """Set the method back."""
        self.factory.buildProtocol = method

I have spent some time on this so let me know if you know about any improvements that I can add my brain does not longer want to know about DirtyReactors…

Leave a comment
Apr 3 12

Manage passwords on Mac OS X

by mandel

Due to a small project I’m developing at the moment I had to access the Mac OS X keyring over python. There is some code of seen around but was not very ‘clean’. The following is my attempt to access the Mac OS X Keyring via python using ctypes, I hope it helps someone out there:

from ctypes import (
    byref,
    c_int32,
    c_uint32,
    c_void_p,
    create_string_buffer,
    memmove,
    cdll,
)
from ctypes.util import find_library
 
from secrets.utils import are_not_none, valid_args
 
 
# Types
 
OSStatus = c_int32
SecKeychainItemRef = c_void_p
SecKeychainRef = c_void_p
 
# Constants
 
errSecSuccess = 0
errSecItemNotFound = -25300
 
 
# Native libraries
 
_security = cdll.LoadLibrary(find_library('Security'))
_core = cdll.LoadLibrary(find_library('CoreServices'))
 
 
def keyring_is_open():
    """Assert that the keyring is not None."""
 
    def keyring_not_none(name, value, method_name='Method'):
        """Ensure that the keyring is open."""
        if not getattr(value, '', None):
            raise OSError('Keyring most be open.')
 
    return valid_args(keyring_not_none, (('Keyring', 0)))
 
 
class Backend(object):
    """Keyring backend for Mac Os X."""
 
    def __init__(self):
        """Create a new Mac OS X backend."""
        super(Backend, self).__init__()
        self.keychain = None
 
    def _get_item(self, realm, username):
        """Return the item that matched the given info."""
        item = SecKeychainItemRef()
        status = _security.SecKeychainFindGenericPassword(self.keychain,
                                         len(realm), realm, len(username),
                                         username, None, None, byref(item))
        return item, status
 
    def open(self):
        """Open the keyring."""
        # we should not be open
        if self.keychain is not None:
            raise OSError()
 
        self.keychain = SecKeychainRef()
        # try to open the keyring
        if _security.SecKeychainOpen('login.keychain', byref(self.keychain)):
            raise OSError("Can't access the keychain")
 
    @keyring_is_open
    def close(self):
        """Close the keyring."""
        _core.CFRelease(self.keychain)
        self.keychain = None
 
    @keyring_is_open
    @are_not_none((
        ('realm', 1),
        ('username', 2),
        ('password', 3)))
    def set_password(self, realm, username, password):
        """Set the password for the given real and username."""
        item, status = self._get_item(realm, username)
 
        if status == errSecItemNotFound:
            status = _security.SecKeychainAddGenericPassword(self.keychain,
                                                len(realm), realm,
                                                len(username), username,
                                                len(password), password, None)
            if status:
                raise OSError()
        else:
            raise OSError()
 
    @keyring_is_open
    @are_not_none((
        ('realm', 1),
        ('username', 2),
        ('password', 3)))
    def update_password(self, realm, username, password):
        """Update the password for the given realm."""
        item, status = self._get_item(realm, username)
 
        if status and status == errSecItemNotFound:
            raise OSError("Can't store password in keychain")
        else:
            status = _security.SecKeychainItemModifyAttributesAndData(item, None,
                                                len(password), password)
            _core.CFRelease(item)
            if status:
                raise OSError()
        if status:
            raise OSError("Can't store password in keychain")
 
    @keyring_is_open
    @are_not_none((
        ('realm', 1),
        ('username', 2)))
    def get_password(self, realm, username):
        """Get the password for the given real and username."""
        length = c_uint32()
        data = c_void_p()
        status = _security.SecKeychainFindGenericPassword(self.keychain,
                                          len(realm), realm,
                                          len(username), username,
                                          byref(length), byref(data), None)
        if status == errSecSuccess:
            password = create_string_buffer(length.value)
            memmove(password, data.value, length.value)
            password = password.raw
            _security.SecKeychainItemFreeContent(None, data)
        elif status == errSecItemNotFound:
            password = None
        else:
            raise OSError("Can't fetch password from system")
        return password
 
    @keyring_is_open
    @are_not_none((
        ('realm', 1),
        ('username', 2)))
    def delete_password(self, realm, username):
        """Delete the password of the given real and username."""
        item, status = self._get_item(realm, username)
        if status == errSecSuccess:
            _security.SecKeychainItemDelete(item)
            _core.CFRelease(item)
        else:
            raise OSError()

I have not added the code of the decorator because they are just noise, the only thing they do is to check that the keyring was indeed opened (self.keyring != None) and that the parameters with the given index are not None (I’m lazy and I prefer to use decorators for this mundane tasks that are done everywhere.

Leave a comment
Apr 2 12

Update about the PyQt twisted reactor

by mandel

In my last post I was trying to explain why you should not be using exec_ from PyQt when using PyQt with its twisted tractor. Because I’m not a big fun of levaing comments open, glyph sent me a very interesting email abut the matter, after asking him for permission here is our conversation which I hope it clarifies the matter much better than I did (othewise I would have not gotten the email :) ):

Hi Mandel,

I noticed your recent blog post, Interesting issues with the PyQt twisted reactor. Since you don’t seem to have comments open to the public, I figured I’d send you some questions and comments and you can update or republish as you see fit.

First of all, Twisted doesn’t include a PyQt reactor. Which one are you using? I’m assuming but there are several copies floating around out there.

Yes, I know that, as far as I know the license used in pyqt does not allow it to be included in twisted. I’m using the one you linked.

You’re describing qtreactor reactor as “buggy” but if there is a bug at all, it is a documentation bug.

Maybe buggy I’d a strong word :)

By using QDialog.exec_(), you are specifically asking Qt to use an alternate event loop, which is to say you are asking it not to process events on the main loop. Twisted schedules its events on the main loop. The design flaw is in Qt (and in many other GUI libraries) which insist on having blocking modal dialog APIs that halt processing of all other events. Bottom line: if you call an API which halts event processing, event processing will be halted :) . This is why you generally should not call blocking functions in Twisted.

Yes, that I know (calling a blocking call within twisted) what I did not expect usa that the deferToThread no to work.

You should really be using the documented API for running a Twisted reactor (reactor.run()) because that does setup required for core Twisted functionality (in particular, spawnProcess) to work correctly. As a bonus, qt4reactor’s reactor.run() will call exec_() on the main application loop for you, and then the rule is very simple: never call exec_ ever. I notice that you’re already calling it (and given that this is a code snippet for the Windows GUI, why is it fenced off in an ‘if sys.platform’ blog?)

Not calling exec_ was indeed the conclusion of the post and I wanted to illustrate the reason rather than saying that it should be done.

but you’re only calling it after exec_() has already exited.

Yes, and that is my “fault” I’m did not expect to be able to have an QEventLoop executing before the QApplication exec.

If you really must call your own exec_ function, see the qt4reactor’s documentation for an explanation of its custom ‘runReturn()’ API.

Nice, I did not know about it.

Finally, you should probably file a documentation patch against qt4reactor on github (assuming that’s the one you’re using) so that others don’t fall into this trap. If you really have reasons to want to use QDialog.exec_() instead of QApplication.exec_(), perhaps you could allow the specification of a custom QEventLoop to qt4reactor.install().

The problem here its not that I want to execute exec but that is “required” in a sense., using a blocking call ensures that the parent window does not receive events. if you think about it that is exactly what you need with modal dialogs.
I hope some of this was helpful,

Very helpful, I did understand the issue although un expected) and I wanted to “document” out so that others don’t fall in it. Nevertheless do you mind if I share your comments, I fear that my blog might has not been clear enough.

-glyph

I hope others find this conversation as useful as I did :) .

PS: I’ll find out how to set up disqus comments in the blog.

Leave a comment
Mar 21 12

Interesting issues with the PyQt twisted reactor

by mandel

On Windows Ubuntu One uses the twisted reactor to run the Qt UI. The main reason for this is that the IPC protocol that is used was written in twisted. This has been giving us a number of head aches like the one we experienced today.

The following code simply shows a dialog that will as the user for his proxy credentials and will store them in the key-ring of which ever platform is being used:

def main():
    """Main method used to show the creds dialog."""
 
    if sys.platform == 'win32':
        import qt4reactor
        qt4reactor.install()
        logger.debug('Qt reactor installed.')
 
    app = QApplication(sys.argv)
    args = parse_args()
    win = ProxyCredsDialog(domain=args.domain,
                           retry=args.retry)
    return_code = win.exec_()
    if sys.platform == 'win32':
        from twisted.internet import reactor
        reactor.run()
    sys.exit(return_code)

From the dialog the most interesting part is the one in which the credentials are stored:

@defer.inlineCallbacks
def _on_save_clicked(self, *args):
    """Save the new credentials."""
    username = unicode(self.ui.username_entry.text()).encode('utf8')
    password = unicode(self.ui.password_entry.text()).encode('utf8')
    creds = dict(username=username, password=password)
    try:
        logger.debug('Save credentials as for domain %s.', self.domain)
        yield self.keyring.set_credentials(self.domain, creds)
    except Exception, e:
        logger.exception('Could not set credentials:')
        self.done(EXCEPTION_RAISED)
    logger.debug('Stored creds')
    self.done(USER_SUCCESS)

And to give even more details, the following is what is used to spawn a thread to store the credentials on windows:

def set_credentials(self, app_name, cred):
    """Set the credentials of the Ubuntu SSO item."""
    # the windows keyring can only store a pair username-password
    # so we store the data using ubuntu_sso as the user name. Then
    # the cred will be stored as the string representation of the dict.
    return deferToThread(self.keyring.set_password, app_name, USERNAME,
                             dumps(cred))

A priori there is nothing wrong with the code, or is it? Doing an IRL test you will see that the credentials are never stored, what’s even more none of the deferreds are called. But why? In theory the qt reactor should be taking care of everything which includes the deferreds, the deferToThread and the execution of the ui.. well, it is not. When we look a little closer we can see that we use the exec_ method from the QDialog and this is the root of the bug. Lets put an example, the following is possible in Qt:

import sys
 
from PyQt4.QtGui import QApplication, QDialog
 
app = QApplication(sys.argv)
dialog = QDialog()
dialog.exec_()

As you can see we are launching the dialog but we did not execute the application, but why is that? The main reason is found in the implementation of exec in the QDialog class (this time in C++, ouch!):

int QDialog::exec()
{
     Q_D(QDialog);
     if (d->eventLoop) {
         qWarning("QDialog::exec: Recursive call detected");
         return -1;
     }
 
     bool deleteOnClose = testAttribute(Qt::WA_DeleteOnClose);
     setAttribute(Qt::WA_DeleteOnClose, false);
 
     bool wasShowModal = testAttribute(Qt::WA_ShowModal);
     setAttribute(Qt::WA_ShowModal, true);
     setResult(0);
 
     show();
 
     QEventLoop eventLoop;
     d->eventLoop = &eventLoop;
     (void) eventLoop.exec();
     d->eventLoop = 0;
 
     setAttribute(Qt::WA_ShowModal, wasShowModal);
 
     int res = result();
     if (deleteOnClose)
         delete this;
     return res;
 }

As you can see the implementation uses a QEventLoop, if you read the documentation you will noticed that using exec of the event loops and the default flag all the events will me processed by this event loop, which is not the main event loop, but a child one (small brain fuck here). This means that, due to the implementation of the qtreactor, when using a dialog exec_ (or better say a QEventLoop.exec method) the reactor main loop is not processing the events which means that all your deferreds, deferToThread etc.. will not be processed :(

Nevertheless there is a way to work around this bug in the qtreactor implementation (because is not the qt fault and it is well documented) which is doing the following workaround:

win = ProxyCredsDialog(domain=args.domain, retry=args.retry)
win.show()
win.finished.connect(exit_app)

The above code ensures that the events will be handeled by the reactor main loop and not by a child QEventLoop.

In summary, qtreactor is buggy and will give you problems.. but if you really have to use it (like we do) do remember this detail, DO NOT use exec.

Leave a comment
Mar 2 12

Liceo frances – the little ones

by mandel

Here you have or little guys playing rugby:

Is in Spanish, sorry, but is worth watching :)

Leave a comment
« Older Entries