GNOME Bugzilla – Bug 677166
Salut personas survive disconnection
Last modified: 2012-06-18 11:48:02 UTC
This is with HEAD of Folks 0.6. (feede0321632b664a981d3df223d7a3724242f5b) - Make sure to have Salut account configured but disabled - Start folks-inspect - persona-stores /org/freedesktop/Telepathy/Account/salut/local_xmpp/account0 (assuming that's the id of your salut Account - No persona is listed as expected - Connect the Salut account - persona-stores /org/freedesktop/Telepathy/Account/salut/local_xmpp/account0 - Some personas are listed (at least your own) - Disconnect the Salut account - persona-stores /org/freedesktop/Telepathy/Account/salut/local_xmpp/account0 - Those personas are still listed ! Furthermore, a Warning is raised (lt-folks-inspect:9586): GLib-CRITICAL **: g_strv_length: assertion `str_array != NULL' failed (gdb) bt full
+ Trace 230287
_tmp7_ = 0x7fffffffd9e8 _tmp7__length1 = 1 _inner_error_ = 0x0 __PRETTY_FUNCTION__ = "folks_inspect_client_main"
I have the same with facebook account: when I disable it, the contacts stay. We should verify if that happens with master as well.
(In reply to comment #0) > This is with HEAD of Folks 0.6. (feede0321632b664a981d3df223d7a3724242f5b) I can reproduce with master as well (03c364f16e9f63967421d8ed526dccf8a26caf13)
This is because the PersonaStore isn’t being removed when its TpAccount is disabled. The code which should be doing that is here: http://git.gnome.org/browse/folks/tree/backends/telepathy/lib/tpf-persona-store.vala#n647 but it isn’t being triggered because at that point, the TpAccount is somehow still enabled and valid. Only later does the TpAccountManager emit account-disabled. I recall being fairly sure that the comment I made when writing that code was correct (i.e. that TpAccount should be set to disabled before the TpConnection is disconnected). Is this not true, or is it a bug in a CM?
(In reply to comment #3) > I recall being fairly sure that the comment I made when writing that code was > correct (i.e. that TpAccount should be set to disabled before the TpConnection > is disconnected). Is this not true, or is it a bug in a CM? I don't think the spec makes that mandatory, so it could even be racy. I would suggest to not depend on event order between Connection and Account since they are from different processes.
(In reply to comment #0) > Furthermore, a Warning is raised That was a separate issue, which I’ve just fixed: commit 0e729d7be7f3431922aa531704aad897c0bb598b Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Jun 18 10:31:54 2012 +0100 telepathy: Set some missing properties on cached TpfPersonas Recently-added properties needed implementing for cached personas. backends/telepathy/lib/tpf-persona.vala | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
Created attachment 216663 [details] [review] Ensure persona stores are removed when disabled https://www.gitorious.org/folks/folks/commits/677166-salut-personas This should fix it.
+1
Sounds like a good candidate for the stable branch as well.
Committed to master. I'll backport it to folks-0-6 if we ever make another 0.6 release. commit 1446c3ed1adafbb153925b81e42e3f199cc1e63a Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Jun 18 11:08:59 2012 +0100 telepathy: Don’t create PersonaStores for disabled accounts We shouldn’t create persona stores for disabled accounts, as they just sit there looking lonely and empty. Persona stores should be created for enabled and valid accounts, and should be removed when those accounts are disabled or become invalid. backends/telepathy/tp-backend.vala | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) commit c8fa98d3ae271c56d7e760f9c2eed37585108541 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Jun 18 11:04:57 2012 +0100 Bug 677166 — Salut personas survive disconnection Handle TpAccounts being disabled by listening for TpAccountManager::account-disabled rather than (erroneously) assuming that TpAccount:enabled will have been changed by the time the corresponding TpConnection is disconnected. This fixes a race condition when accounts are disabled, where the account’s personas would persist if TpAccount:enabled hadn’t changed by the time the connection was disconnected. This comes at the cost of potentially storing and re-loading the set of personas for that account to the cache, only to later delete the cache file when TpAccount:enabled changes. I can’t think of a simple fix for this. Closes: https://bugzilla.gnome.org/show_bug.cgi?id=677166 NEWS | 3 +- backends/telepathy/lib/tpf-persona-store.vala | 50 ++++++++++++++---------- 2 files changed, 31 insertions(+), 22 deletions(-)