[Bug 27676] ContactInfo high-level API

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=27676

--- Comment #22 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-06-02 10:35:53 PDT ---
> +      G_OBJECT (self));

Passing the TpContact through the tp_cli call as the weak_object, when we know
that it's reffed by the simple result and thus shouldn't be finalized during
the call, at least deserves a comment.

When doing this in conjunction with a TpProxyPendingCall, I'd be inclined to
say that it's risky to have a weak_object (cancelling manually after a
weak_object might have died is a use-after-free); just put the TpContact in the
context struct, which you have anyway.

> +  tp_proxy_pending_call_cancel (data->call);

I think this should be protected by "if (data->call != NULL)"...

>  contact_info_request_cb (TpConnection *connection,

... and this should set data->call to NULL on entry.

> +          g_cancellable_disconnect (data->cancellable, data->cancellable_id);

The docs say: "Additionally, in the event that a signal handler is currently
running, this call will block until the handler has finished. Calling this
function from a "cancelled" signal handler will therefore result in a
deadlock."
tp_proxy_pending_call_cancel results in the pending call's data being freed,
and its destructor calls g_cancellable_disconnect.

However, tp_proxy_pending_call_cancel frees the pending call in an idle because
of Bug #14750, which saves us from this potential bug.

I think this is OK, but very very subtle: it deserves a comment in
connection-contact-info.c saying that this code only works because of the fix
for Bug #14750, and tp_proxy_pending_call_cancel() should gain a comment noting
that this method relies on the idle.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.