Re: [java-gnome-hackers] Ugly deadlock with Dialog.run()

[Andrew C. <an...@op...>]

On Mon, 2010-08-09 at 21:42 +0200, Vreixo Formoso wrote:
> I would like to discuss with you an ugly behavior related with the way
> java-gnome automatically handles the Gtk thread-aware feature. I don't
> really think this is a bug, but I would like to talk about it.

Yeah, I raised this on 13 Jan,
http://article.gmane.org/gmane.comp.gnome.bindings.java.devel/1436
and
https://bugzilla.gnome.org/show_bug.cgi?id=606796

It is pretty ugly.

> I've working for several years with java-gnome 

Well, you ARE the #2 committer :)
http://www.ohloh.net/p/java-gnome/contributors


> There is, however, one ugly case. When you call "blocking" methods such
> as Gtk.main() or Dialog.run(), Gtk+ takes care of releasing the lock,
> which makes possible to invoke gtk functions from another thread while
> the main thread is waiting on Gtk.main(). It works very well, except in
> one case: calling Dialog.run() inside a signal handler. It is a common
> situation, when you have to temporally show a dialog in response to a
> user action (e.g. clicking a Button). 
> 
> In this case, when you call run(), the calling thread already holds the
> lock (it is running in a signal handler). Internally, java-gnome
> re-acquires the lock before calling native gtk_dialog_run(). It also
> does it with every other function, and it works ok because java lock are
> re-entrant. However, differently from other functions, gtk_dialog_run()
> will wait in an event loop until the user closes the dialog. As I said
> above, Gtk+ knows that, so it will release the lock, but the problem is
> that we have acquired the lock twice, so the lock keeps owned by the
> calling thread....

I agree with your analysis.

It's not so much "that we have acquired the lock twice" so much as it is
that the gtk_dialog_run() code assumes it can unlock once and be clear.

> What do you think about? Is this ok? Could we (maybe) add a
> runWithoutLock() function? 

[please read my other email]

We could

1. Take our own lock (which we could then manually unlock in special
cases)

2. Just mess with a manual MonitorExit/MonitorEnter

or

3. Seriously mess with the entire main loop process:

        If *we* were iterating the main loop manually, then we could
        control when it takes our lock and when it doesn't. The problem
        is replacing gtk_main() with gdk_events_pending() &
        gdk_main_iteration_do() is NOT really the right thing to do.


All of these are invasive. #2 is probably easiest, but there's a bigger
problem: what happens if you're not nested 2 deep, but nested 3 deep?
Then it won't help.

> Note that in my opinion is it ok as it is now, you can always forget
> about run() 

I don't have any hard opinions about it. It's a bug, and we need to
either fix it if we can. I'd like to keep run(), though. It's nice to
use inside signal handlers. Or it would be if it worked :)

AfC
Sydney