Bug 27242

Summary:	tp_g_signal_connect_object: Invalid read in object_remove_closure when observer is destroyed
Product:	Telepathy	Reporter:	Guillaume Desmottes <guillaume.desmottes>
Component:	tp-glib	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED NOTOURBUG	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium
Version:	git master
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Guillaume Desmottes 2010-03-22 07:41:19 UTC

To reproduce:
- Get http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/connect-weak-613583
- Start Empathy
- Stop Empathy
- Empathy crashes because of this error

==13124== Invalid read of size 4
==13124==    at 0xD79237F: object_remove_closure (gobject.c:3017)
==13124==    by 0xD79046C: g_closure_invalidate (gclosure.c:269)
==13124==    by 0xD79023C: g_closure_unref (gclosure.c:589)
==13124==    by 0xD79F98C: handler_unref_R (gsignal.c:637)
==13124==    by 0xD79FD37: g_signal_handler_disconnect (gsignal.c:2384)
==13124==    by 0x5F66B62: observer_destroyed_cb (util.c:896)
==13124==    by 0xD791DDF: weak_refs_notify (gobject.c:1992)
==13124==    by 0xDBF31B1: g_datalist_clear (gdataset.c:120)
==13124==    by 0xD792550: g_object_unref (gobject.c:2441)
==13124==    by 0x43F0CE: main (empathy.c:737)
==13124==  Address 0x8 is not stack'd, malloc'd or (recently) free'd

This used to work fine using empathy_signal_connect_weak so I guess that's a tp-glib issue.

Comment 1 Simon McVittie 2010-03-22 09:17:48 UTC

From your backtrace, it appears this happens when the signal-receiver (the "observer" in the terminology used in that code) loses its last reference, this causes disconnection of the signal, and that causes the closure to be invalidated, which somehow causes the closure to be removed from an object that doesn't exist (it's unclear whether that object was meant to be the observer or the emitter).

I've been unable to cause this situation in a simple test case, using GLib 2.22.4-1 (Debian unstable) and telepathy-glib git master. What version of GLib are you using, and is it possible that it's buggy or behaves differently?

Could you try building and running tests/signal-connect-object.c from this branch? It works for me, but if it fails for you then that might be informative...

http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/signal-connect-test

Comment 2 Guillaume Desmottes 2010-03-23 00:53:28 UTC

This test works fine for me. I'm using Glib 2.22.3-0ubuntu1 (Ubuntu Karmic). Did you try using my Empathy branch?

Comment 3 Simon McVittie 2010-04-06 10:41:23 UTC

It's scary. The problem appears to be related to invalidating a GClosure that points to an object (by disconnecting from a signal), from that object's own finalize function. Unfortunately, that's sort of the point of this branch...

Still looking into it, but it won't be fixed today.

Comment 4 Simon McVittie 2010-04-07 05:32:32 UTC

In the Empathy branch, the (first) problematic case is an EmpathyLocationManager watching TpAccount::status-changed.

At the boundary between stack frames 10 and 11, location manager's dispose() incorrectly chains up to GObject's finalize(). This causes misc stuff to be freed before the weak ref notifications have necessarily run, leading to disaster.

Comment 5 Simon McVittie 2010-04-07 05:49:51 UTC

Fixing the Empathy bug seems to fix this, so, not telepathy-glib's bug.

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.