After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 627242 - Some contacts stay in the contact list once disconnected
Some contacts stay in the contact list once disconnected
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Meta Contacts
2.29.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
: 626411 (view as bug list)
Depends on:
Blocks: 626599
 
 
Reported: 2010-08-18 08:41 UTC by Guillaume Desmottes
Modified: 2010-08-27 10:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't filter out removals of re-linked Individual at the IndividualManager (2.08 KB, patch)
2010-08-27 07:59 UTC, Travis Reitter
committed Details | Review

Description Guillaume Desmottes 2010-08-18 08:41:37 UTC
When I change my presence to offline most of my contacts disappear from the contacts list as expected but a few stay there (their presence is unchanged).
Not sure if it's related but something is going wrong, I have a lot of this kind of warnings:

(empathy:25893): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

(empathy:25893): GLib-GObject-WARNING **: instance of invalid non-instantiatable type `(null)'

(empathy:25893): GLib-GObject-CRITICAL **: g_signal_handlers_disconnect_matched: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed

Unfortunatelly I can't easily get a trace for those because I have a shit load of
(empathy:25893): TelepathyBackend-WARNING **: tpf-persona.vala:265: Group invalidated: User requested disconnection
warnings before so it crashed before if I use fatal-criticals.
Comment 1 Guillaume Desmottes 2010-08-18 09:17:00 UTC
For the record, I send a log of this issue to Travis and Philip by mail.
Comment 2 Travis Reitter 2010-08-20 01:01:19 UTC
(In reply to comment #0)
> When I change my presence to offline most of my contacts disappear from the
> contacts list as expected but a few stay there (their presence is unchanged).

I can reproduce this some of the time; sometimes the list is empty as expected.

And some of the times I get the warnings below, but can continue along; other times, it ends up in a full-out crash (due to a double-free).

Neither of these problems happen when I have zero linked contacts.

> Not sure if it's related but something is going wrong, I have a lot of this
> kind of warnings:
> 
> (empathy:25893): GLib-GObject-CRITICAL **: g_object_unref: assertion
> `G_IS_OBJECT (object)' failed

This is due to TpfPersonaStore. When the TpConnection becomes disconnected, we reset some internal state in the TpfPersonaStore.

This ends up finalizing a GeeHashMap (which maps TpHandles to TpfPersonas), which frees its keys and values. The problem is that, in the case above, the TpfPersona seems to be disposed early.

I suspect there's something in Empathy that's excessively unreffing the persona. Of course, there are so many things that hold references to the personas that this is turning out to be very difficult to debug. But I'm very confident it's in Empathy, not Folks (after combing through the generated C code for Folks very carefully).

> (empathy:25893): GLib-GObject-WARNING **: instance of invalid
> non-instantiatable type `(null)'
> 
> (empathy:25893): GLib-GObject-CRITICAL **:
> g_signal_handlers_disconnect_matched: assertion `G_TYPE_CHECK_INSTANCE
> (instance)' failed
> 
> Unfortunatelly I can't easily get a trace for those because I have a shit load
> of
> (empathy:25893): TelepathyBackend-WARNING **: tpf-persona.vala:265: Group
> invalidated: User requested disconnection
> warnings before so it crashed before if I use fatal-criticals.

These shouldn't show up in Folks >= 0.1.14.1 (though I couldn't reproduce these in the first place).
Comment 3 Philip Withnall 2010-08-20 11:34:28 UTC
This double-freeing of Personas may be happening because your linked Individuals contain duplicate Personas, but the duplicates are only duplicate entries (i.e. two list elements referring to the same object instance, so they're effectively sharing a reference).

I fixed this here: http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/626544-im-address-duplicates so you might want to see if that fixes the problem.
Comment 4 Guillaume Desmottes 2010-08-20 13:44:26 UTC
I tried your branch and got this error when disconnecting:


Lib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed
aborting...

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff49436c6 in g_logv (log_domain=0x7ffff4e594cf "GLib-GObject", log_level=G_LOG_LEVEL_CRITICAL, format=0x7ffff49bcb18 "%s: assertion `%s' failed", 
    args1=0x7fffffffd470) at gmessages.c:544
544			G_BREAKPOINT ();
(gdb) bt
  • #0 g_logv
    at gmessages.c line 544
  • #1 g_log
    at gmessages.c line 568
  • #2 g_return_if_fail_warning
  • #3 g_object_unref
    at gobject.c line 2514
  • #4 gee_hash_map_real_clear
    at hashmap.vala line 234
  • #5 gee_abstract_map_clear
    at abstractmap.vala line 113
  • #6 gee_hash_map_finalize
    at hashmap.vala line 273
  • #7 g_object_unref
    at gobject.c line 2580
  • #8 tpf_persona_store_reset
    at tpf-persona-store.c line 520
  • #9 tpf_persona_store_account_status_changed_cb
    at tpf-persona-store.c line 1008
  • #10 _tpf_persona_store_account_status_changed_cb_tp_account_status_changed
    at tpf-persona-store.c line 616
  • #11 _tp_marshal_VOID__UINT_UINT_UINT_STRING_BOXED
    at _gen/signals-marshal.c line 2842
  • #12 g_closure_invoke
    at gclosure.c line 766
  • #13 signal_emit_unlocked_R
    at gsignal.c line 3252
  • #14 g_signal_emit_valist
    at gsignal.c line 2983
  • #15 g_signal_emit
    at gsignal.c line 3040
  • #16 _tp_account_update
    at account.c line 565
  • #17 _tp_cli_account_invoke_callback_for_account_property_changed
    at ../telepathy-glib/_gen/tp-cli-account-body.h line 94
  • #18 tp_proxy_signal_invocation_run
    at proxy-signals.c line 266
  • #19 g_idle_dispatch
    at gmain.c line 4224
  • #20 g_main_dispatch
    at gmain.c line 2119
  • #21 g_main_context_dispatch
    at gmain.c line 2672
  • #22 g_main_context_iterate
    at gmain.c line 2750
  • #23 g_main_loop_run
    at gmain.c line 2958
  • #24 gtk_main
    at gtkmain.c line 1203
  • #25 main
    at empathy.c line 548

Comment 5 Travis Reitter 2010-08-20 15:31:21 UTC
I've got the same problem as Guillaume as well - that branch doesn't seem to affect this bug (though there could be a related problem in some other code).
Comment 6 Philip Withnall 2010-08-20 15:40:34 UTC
(It wasn't meant to; I just thought it might help when I saw this bug.)
Comment 7 Travis Reitter 2010-08-27 07:59:09 UTC
Created attachment 168861 [details] [review]
Don't filter out removals of re-linked Individual at the IndividualManager

This patch is the squashed (though already 1-commit) version of the changes in this branch:
http://git.collabora.co.uk/?p=user/treitter/empathy.git;a=shortlog;h=refs/heads/bgo627242

The net effect of this branch is to have the IndividualManager signal the removal of re-linked Individuals who are removed by a Telepathy account going offline.

The existing code should have already supported this, except a strange phenomenon prevented it from working properly:

IndividualManager->priv->individuals stores a mapping of {Individual id: Individual}. Upon Connection status change to DISCONNECTED, priv->individuals would contain only the second-last-to-be-removed Individual ("Individual X") at the time it was trying to remove the last (re-linked) Individual ("Individual Y"). Since the Individual Y didn't exist in priv->individuals, it would be filtered out, and the IndividualStore would not receive the signal to remove that Individual.

Next, the IndividualManager would try to remove Individual X, but priv->individuals would only contain Individual Y.

In the end, all the correct Individuals would be removed from priv->individuals.

The mix up in these removals is very odd, and I honestly can't explain it (I pored over the code quite a bit, but nothing stood out).

In the end, this patch seems to fix this bug and seems relatively safe, so I think it's worth merging. If you would like to do so before the next release (in case I'm not available), please merge it yourself (assuming you approve it).
Comment 8 Philip Withnall 2010-08-27 09:56:03 UTC
Looks reasonable to me.

commit 17d384008361384be53a2466fcd9a7e837e10d09
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Thu Aug 26 16:39:42 2010 -0700

    Don't filter out Individual removal at the IndividualManager level.
    
    The IndividualManager sometimes falsely filters out members of
    FolksIndividualAggregator:individuals-changed:removed that should be passed
    along to its users.

 libempathy/empathy-individual-manager.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)
Comment 9 Guillaume Desmottes 2010-08-27 10:08:06 UTC
*** Bug 626411 has been marked as a duplicate of this bug. ***