GNOME Bugzilla – Bug 767299
Adding a Telepathy account is racy after adding a headerbar
Last modified: 2017-04-08 19:15:56 UTC
Ever since we changed the dialog to use a header bar (bug 729638), adding a Telepathy account became racy. The code looks like this: * create TpawAccoutWidget inside a GtkDialog * wait for gtk_dialog_run to return * if the GoaObject is not yet available, run a GMainLoop to wait for it The original pre-headerbar code assumed that TpawAccountWidget::close will be emitted once the account has been created and ready to use. The TpawAccountWidget::close handler in turn emitted GtkDialog::response. It then used a hand rolled GMainLoop to wait for GoaTpAccountLinker to create the corresponding GoaObject. Note that TpawAccountWidget doesn't just create the TpAccount. It does some more asynchronous work with it. eg., enabling it. On the other hand, GoaTpAccountLinker starts creating the GoaObject immediately when the TpAccount is created. Therefore, there is no guarantee that TpawAccountWidget::close will be emitted before GoaTpAccountLinker has created the corresponding GoaObject. Now, post-headerbar, GtkDialog::response is emitted immediately when the user clicks one of the response buttons, and we return from gtk_dialog_run. At this point the TpAccount hasn't even been created. Once it is ready, TpawAccountWidget::close (and a second GtkDialog::response) would follow, but there is nothing to match it. We are relying on TpawAccountWidget::close to win the race against GoaTpAccountLinker. If it loses the race, then the GMainLoop will be quit while the dialog is still working, and lead to a memory error. Surprisingly, at least on my system, we were winning the race all the time, until recently. eg., one git bisect run pointed at commit 3dfcd3c4df589db2 as the faulty commit, which doesn't make sense.
Created attachment 329211 [details] [review] telepathy: Simplify the code by re-using the existing GoaClient
Created attachment 329212 [details] [review] telepathy: Remove the unused GCancellable
Created attachment 329213 [details] [review] telepathy: Make the GoaObject code resemble its TpAccount counterpart
Created attachment 329214 [details] [review] telepathy: Fix a race between TpawAccountWidget and GoaTpAccountLinker
Related: bug 767295
Review of attachment 329213 [details] [review]: I don't like this change anymore because: ::: src/goabackend/goatelepathyprovider.c @@ +347,3 @@ + + g_assert (data->goa_object == NULL); + data->goa_object = g_object_ref (goa_object); We can't be sure that nobody is adding a Telepathy account behind our back. If that happens, then we can't assert that data->goa_object is NULL.
Review of attachment 329213 [details] [review]: Let's drop this.
Created attachment 329294 [details] [review] telepathy: Fix a race between TpawAccountWidget and GoaTpAccountLinker
(In reply to Debarshi Ray from comment #0) > Surprisingly, at least on my system, we were winning the race all the time, > until recently. eg., one git bisect run pointed at commit 3dfcd3c4df589db2 > as the faulty commit, which doesn't make sense. The Telepathy provider used to store empty keyring entries. It could be that the change to not do that (ie. commit 3dfcd3c4df589db2) changed the outcome of the race.