Bug 52441 - Race when preparing blocked contacts
Summary: Race when preparing blocked contacts
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-24 11:52 UTC by Guillaume Desmottes
Modified: 2013-09-12 18:54 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
tp_connection_upgrade_contacts(): Use special trick of ready_enough_for_contacts (1.71 KB, patch)
2012-07-24 13:30 UTC, Xavier Claessens
Details | Splinter Review

Description Guillaume Desmottes 2012-07-24 11:52:24 UTC
Original bug: https://bugzilla.gnome.org/show_bug.cgi?id=679825#c7


tp-glib tries to prepare blocked contact while the CONNECTED feature is not prepared yet; raising a warning.

(empathy:4339): tp-glib-CRITICAL **: tp_connection_upgrade_contacts: assertion
`tp_proxy_is_prepared (self, TP_CONNECTION_FEATURE_CONNECTED)' failed

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff56d3e29 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
(gdb) bt


#0  0x00007ffff56d3e29 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#1  0x00007ffff56d3fc2 in g_log () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff7152daa in tp_connection_upgrade_contacts (
    self=self@entry=0x7bf750, n_contacts=n_contacts@entry=4, 
    contacts=contacts@entry=0xc6aad0, n_features=n_features@entry=16, 
    features=<optimized out>, 
    callback=callback@entry=0x7ffff714d490 <upgrade_contacts_fallback_cb>, 
    user_data=0xc6c2e0, destroy=0x415190 <g_object_unref@plt>, 
    weak_object=weak_object@entry=0x0) at contact.c:4311
#3  0x00007ffff7152eb1 in tp_connection_upgrade_contacts_async (
    self=self@entry=0x7bf750, n_contacts=n_contacts@entry=4, 
    contacts=contacts@entry=0xc6aad0, n_features=16, features=<optimized out>, 
    callback=callback@entry=0x7ffff71a4280 <upgrade_contacts_cb>, 
    user_data=user_data@entry=0xc6c550) at contact.c:4761
#4  0x00007ffff71a5aab in tp_simple_client_factory_upgrade_contacts_async (
    self=<optimized out>, connection=0x7bf750, n_contacts=4, 
    contacts=0xc6aad0, callback=<optimized out>, user_data=<optimized out>)
    at simple-client-factory.c:924
#5  0x00007ffff714787e in process_queued_blocked_changed (self=0x7bf750)
    at connection-contact-list.c:1784
#6  0x00007ffff71258b3 in _tp_cli_connection_interface_contact_blocking_invoke_callback_request_blocked_contacts (weak_object=<optimized out>, 
    user_data=<optimized out>, 
---Type <return> to continue, or q <return> to quit---
    generic_callback=0x7ffff71479d0 <request_blocked_contacts_cb>, 
    args=0xc81ac0, error=0x0, self=0x7bf750)
    at _gen/tp-cli-connection-body.h:7963
#7  _tp_cli_connection_interface_contact_blocking_invoke_callback_request_blocked_contacts (self=0x7bf750, error=0x0, args=0xc81ac0, 
    generic_callback=0x7ffff71479d0 <request_blocked_contacts_cb>, 
    user_data=<optimized out>, weak_object=<optimized out>)
    at _gen/tp-cli-connection-body.h:7946
#8  0x00007ffff71a0c90 in tp_proxy_pending_call_idle_invoke (p=0xc1bd20)
    at proxy-methods.c:155
#9  0x00007ffff56ccc65 in g_main_context_dispatch ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007ffff56ccf98 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007ffff56cd054 in g_main_context_iteration ()
   from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00007ffff5c6c72c in g_application_run ()
   from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#13 0x0000000000415d50 in main (argc=1, argv=0x7fffffffe258) at empathy.c:847
Comment 1 Xavier Claessens 2012-07-24 12:26:00 UTC
The problem here is RequestBlockedContacts returns before TP_CONNECTION_FEATURE_CONNECTED is prepared. This is probably because the CM is actually connected but TpProxy does more extra stuff before marking the CONNECTED feature.

Possible fix for this is to make TP_CONNECTION_FEATURE_CONTACT_BLOCKING feature depend on TP_CONNECTION_FEATURE_CONNECTED, so preparing BLOCKING will wait until connection goes online. This has a side effect that if you naïvely add that feature on the factory, TpAccount will never signal the connection until it's done connecting...

Another solution (and that's what CONTACT_LIST feature does) is to just immediately return success if the connection is not yet connected, then emit "blocked-contacts-changed" signal with the initial set once the connection is connected.
Comment 2 Simon McVittie 2012-07-24 12:29:29 UTC
(In reply to comment #1)
> This has a side effect that if you naïvely add
> that feature on the factory, TpAccount will never signal the connection until
> it's done connecting...

I don't like this.

> Another solution (and that's what CONTACT_LIST feature does) is to just
> immediately return success if the connection is not yet connected

I prefer this one.
Comment 3 Xavier Claessens 2012-07-24 13:30:30 UTC
Created attachment 64613 [details] [review]
tp_connection_upgrade_contacts(): Use special trick of ready_enough_for_contacts

TpConnection itself wants to prepare TpContacts before being officially
CONNECTED. This was already the case for self contact. Use the same
trick for blocked contacts.
Comment 4 Xavier Claessens 2012-07-24 13:32:30 UTC
After digging, it seems it's actually easier than I though. We already has such trick for the self contact which uses the _by_handle() path, now do the same for blocked contacts which uses the _upgrade_contacts() path. 

The idea is that TpConnection itself can prepare contacts before being officially CONNECTED.
Comment 5 Guillaume Desmottes 2012-07-24 13:52:46 UTC
Comment on attachment 64613 [details] [review]
tp_connection_upgrade_contacts(): Use special trick of ready_enough_for_contacts

Review of attachment 64613 [details] [review]:
-----------------------------------------------------------------

Looks good but shouldn't we do the same in tp_connection_get_contacts_by_id() ?
Comment 6 Xavier Claessens 2012-07-24 14:00:49 UTC
_by_id() should not be used internally, it is (or should be) used only for IDs typed by the user in the UI.
Comment 7 Guillaume Desmottes 2012-07-24 14:11:12 UTC
I see; ++
Comment 8 Xavier Claessens 2012-07-24 14:19:31 UTC
Merged in both 0.18 and master.
Comment 9 Simon McVittie 2013-09-12 18:54:12 UTC
telepathy-glib-0.18.2, telepathy-glib-0.19.5


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.