[Sugar-devel] [PATCH sugar] Make sure the connection is established before we use it SL #3119

[Sascha Silbe]

Excerpts from Simon Schampijer's message of 2011-10-07 19:49:08 +0200:

> The code is a bit racy. We need to make sure the connection is
> already established to not get the traceback. Tested with
> Salut and Gabble.

When reading "git log", there's no Traceback, so it's not clear what the
above refers to. How about this instead:

The Connection property [1] can get set while the account manager is
still trying to connect [2,3]. We need to wait for the connection to be
actually established (ConnectionStatus=Connected) before we can use it.


[src/jarabe/model/neighborhood.py]
> @@ -251,7 +251,10 @@ class _Account(gobject.GObject):
>              self._check_registration_error()
>              self._connection = None
>          elif self._connection is None:
> -            self._prepare_connection(properties['Connection'])
> +            if 'ConnectionStatus' not in properties:
> +                return
> +            if properties['ConnectionStatus'] == CONNECTION_STATUS_CONNECTED:
> +                self._prepare_connection(properties['Connection'])

I'm not sure your patch is kosher. Right now we're using only the
Connection property and explicitly check whether it's part of the
properties that (might) have changed (see the first few lines of
__account_property_changed_cb()).

Your patch adds ConnectionStatus to the mix (probably for a good
reason). Since AccountPropertyChanged [4] is not required to include
unchanged properties (but it may, which is probably why it works for
you) and the Connection property is likely to get set before we're
actually connected (probably the reason for your change), there's a
chance we don't recognise the connection got established.

Maybe we should change the code to trigger on ConnectionStatus instead
of Connection and explicitly query for Connection if it's not included
in properties?

Sascha

[1] http://telepathy.freedesktop.org/spec/Account.html#Property:Connection
[2] http://telepathy.freedesktop.org/spec/Account.html#Property:ConnectionStatus
[3] http://telepathy.freedesktop.org/spec/Connection.html#Enum:Connection_Status
[4] http://telepathy.freedesktop.org/spec/Account.html#Signal:AccountPropertyChanged
-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20111008/dfcb0c93/attachment.pgp>