race condition in idle handling (#321886)

[Owen Taylor <otaylor redhat com>]

On Tue, 2005-11-22 at 12:02 -0500, Matthias Clasen wrote:
> The meeting is intended for the GTK+ team, but everybody is 
> welcome to come and listen. The meeting logs will be posted 
> on the GTK+ website (http://www.gtk.org/plan/meetings).
> 
>  Place: irc.gnome.org:#gtk-devel
>  Time: 21:00 UTC (16:00 EDT), Tuesday, November 22
> 
> Possible agenda items:
> - race condition in idle handling (#321886)

The second patch on the bug is buggy because it checks the destroyed
flag on the source *without holding the corresponding lock*.

It cannot be fixed up be moving the LOCK_CONTEXT()/UNLOCK_CONTEXT(),
because you must not run any user code, and in particular must
not acquire the GTK+ lock while holding the main loop lock. 

You could fix it by *relocking* the context in order to check the
destroyed flag on the source. I think the idea of 
g_source_set_lock_funcs() is pretty ugly though ... 

You could see this bug as being a symptom of a larger problem with
using GObject from a threaded program: the weak reference facility
provided by g_object_weak_ref() and g_object_add_weak_pointer() 
is inherently not thread safe - you need the operation to
atomically:

 Checking whether the pointer is null
 If not null, reference the object

This isn't really a big problem for this bug because we can make
weak references (implemented here in finalize) thread safe by just
surrounding them by the global GTK+ lock. But it might be worth
taking a look at whether it would make sense to look at the general
problem.

In other words, if you fixed the thread safety of 
g_cclosure_new_object(), then you could use that and 
g_source_set_closure() and not have to worry about removing the 
sources in finalize.

I don't know how hard that would be ... it might be a significant
challenge.

Regards,
						Owen

To throw out another random idea, you could use thread-local-data
and have:

 g_source_is_current_destroyed()

or some better name for that. Or alternatively, and somewhat 
horribly, you could invoke idles and timeouts with a hidden extra
parameter which was the GSource and have g_source_is_destroyed().