[Bugs] #2964 UNSP: Race condition while buddy initiation

[Sugar Labs Bugs]

#2964: Race condition while buddy initiation
------------------------------------------+---------------------------------
    Reporter:  alsroot                    |          Owner:  erikos                     
        Type:  defect                     |         Status:  assigned                   
    Priority:  Unspecified by Maintainer  |      Milestone:  Unspecified by Release Team
   Component:  sugar                      |        Version:  Git as of bugdate          
    Severity:  Unspecified                |       Keywords:                             
Distribution:                             |   Status_field:  Unconfirmed                
------------------------------------------+---------------------------------
Changes (by sascha_silbe):

  * owner:  => erikos
  * status:  new => assigned
  * distribution:  Unspecified =>


Comment:

 We should probably call [http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.SetProperties SetProperties]
 right after creating the connection in
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line256
 jarabe.model.neighborhood._Account._prepare_connection()], not in
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line261
 __connection_ready_cb()] after the connection has been established.

 Nevertheless Sugar should cope with this race condition on the receiver
 side. As PEP is used we should get notified of the properties as soon as
 the sender publishes them ([http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.PropertiesChanged
 PropertiesChanged] signal,
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line401
 __buddy_info_updated_cb()]).

 If I grok the code right, this should be race-free: The signal callbacks
 get set up in
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line335
 __get_self_handle_cb()], as part of the callback for the {{{Status}}} call
 in
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line268
 __connection_ready_cb()]. The [http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.GetProperties GetProperties] call
 happens in
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line440
 _update_buddy_activities()] and
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line577
 __get_contact_attributes_cb()], both times only in response to the signals
 we subscribed to in
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line335
 __get_self_handle_cb()]:

  *
 [http://telepathy.freedesktop.org/spec/Channel_Interface_Group.html#Signal:MembersChanged
 MembersChanged] signal ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line526
 __members_changed_cb()] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line540
 _add_buddy_handles()] ->
 [http://telepathy.freedesktop.org/spec/Connection_Interface_Contacts.html#Method:GetContactAttributes
 GetContactAttributes] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line548
 __get_contact_attributes_cb()]
  * [http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.ActivitiesChanged
 ActivitiesChanged] signal ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line422
 __buddy_activities_changed_cb()]
  * [http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.ActivityProperties.ActivityPropertiesChanged
 ActivityPropertiesChanged] signal ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line504
 __activity_properties_changed_cb()] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line518
 _update_activity()] -> [http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.GetActivities GetActivities] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line607
 __got_activities_cb()]
  *
 [http://telepathy.freedesktop.org/spec/Channel_Interface_Group.html#Signal:MembersChanged
 MembersChanged] signal ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line526
 __members_changed_cb()] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line540
 _add_buddy_handles()] ->
 [http://telepathy.freedesktop.org/spec/Connection_Interface_Contacts.html#Method:GetContactAttributes
 GetContactAttributes] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line548
 __get_contact_attributes_cb()] -> [http://people.collabora.com/~cassidy
 /spec-olpc.html#org.laptop.Telepathy.BuddyInfo.GetActivities
 GetActivities] ->
 [https://git.sugarlabs.org/sugar/mainline/blobs/master/src/jarabe/model/neighborhood.py#line607
 __got_activities_cb()]

 We should consider the use case of non-Sugar buddies when fixing the
 receiver side of this bug: When [http://people.collabora.com/~cassidy
 /spec-olpc.html#org.laptop.Telepathy.BuddyInfo.GetProperties
 GetProperties] fails, it can be due to this bug
 ([http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.GetProperties SetProperties]
 called too late on the sender side) or because the buddy isn't running
 Sugar. We should be able to distinguish these two cases by examining the
 [http://xmpp.org/extensions/xep-0115.html capabilities] (see
 [http://xmpp.org/extensions/xep-0163.html#approach-filter PEP: Filtered
 Notifications]). In the non-Sugar case we should show the buddy right
 away. For Sugar buddies we might want to wait for the
 [http://people.collabora.com/~cassidy/spec-
 olpc.html#org.laptop.Telepathy.BuddyInfo.PropertiesChanged
 PropertiesChanged] signal, potentially forever.

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/2964#comment:2>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system