GNOME Bugzilla – Bug 745248
Re-entancy issues with GTK event API called from Clutter event handlers
Last modified: 2015-09-03 06:15:49 UTC
Hi, So I jhbuilded maps from scratch for the first time in a while today. And found that Maps will freeze completly when clicking the Map. I bisected clutter (which was a hassle because of libinput API break) and this is what the bisect said: 60dbeb9425f51fc602ba2fe89b2a968ef4b527ed is the first bad commit commit 60dbeb9425f51fc602ba2fe89b2a968ef4b527ed Author: Emmanuele Bassi <ebassi@gnome.org> Date: Sun Jan 11 16:48:47 2015 +0000 Try again at using the GDK backend by default We tried once with commit 398a7ac7 and ended up reverting because of regressions in the input layer and on Wayland. We should try again, now that those regressions have been fixed. https://bugzilla.gnome.org/show_bug.cgi?id=734587 Before that commit clicking the map works fine.
Ok, so for shits and giggles I removed the line in Maps that did: this.view.reactive = true; And after that I could move the map around, but I got other strange effects. But maybe that is a clue?
The strange effect being that the map bubbles did not dissapear while panning, which might be because the view is not longer reactive. Since it listens on button-press-event on the ChamplainView clutter actor.
Some more debug, I looked in clutter-gtk, and I did this: diff --git a/clutter-gtk/gtk-clutter-embed.c b/clutter-gtk/gtk-clutter-embed.c index 64cc8b6..78de2d1 100644 --- a/clutter-gtk/gtk-clutter-embed.c +++ b/clutter-gtk/gtk-clutter-embed.c @@ -954,8 +954,11 @@ gtk_clutter_embed_event (GtkWidget *widget, GdkEvent *event) { #if defined(CLUTTER_WINDOWING_GDK) + g_print("clutter windowing gdk\n"); if (clutter_check_windowing_backend (CLUTTER_WINDOWING_GDK)) clutter_gdk_handle_event (event); + + g_print("handled event\n"); #endif And noticed that after I pressed it never printed handled event, (I saw it before). Then I added a bunch of debug in clutter-gdk-event.c, the most interesting being: diff --git a/clutter/gdk/clutter-event-gdk.c b/clutter/gdk/clutter-event-gdk.c index 9f7dec7..d68635c 100644 --- a/clutter/gdk/clutter-event-gdk.c +++ b/clutter/gdk/clutter-event-gdk.c @@ -103,8 +103,9 @@ clutter_gdk_handle_event (GdkEvent *gdk_event) if (stage == NULL) return GDK_FILTER_CONTINUE; + g_print ("acquire lock\n"); _clutter_threads_acquire_lock (); - + g_print ("lock aquired\n") switch (gdk_event->type) { case GDK_DELETE: And the output from Maps: [...] clutter windowing gdk acquire lock lock aquired handled event clutter windowing gdk acquire lock lock aquired handled event clutter windowing gdk acquire lock lock aquired handled event Gjs-Message: JS LOG: button-press-event Gjs-Message: JS LOG: button-press-event clutter windowing gdk acquire lock [freezes here]
Could launch it in gdb and print the backtrace after the lock has appear? It looks like something is doing something weird with the mainloop.
Here we go : ^C Program received signal SIGINT, Interrupt. syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 38 ../sysdeps/unix/sysv/linux/x86_64/syscall.S: No such file or directory. (gdb) bt
+ Trace 234725
Created attachment 298084 [details] [review] mainWindow: prevent Clutter deadlock This is a pretty ugly workaround.
Created attachment 298085 [details] [review] mainWindow: prevent Clutter deadlock This is a pretty ugly workaround.
Hi Lionel, thanks for looking into this! I don't quite understand tho. This implies that Maps is doing something wrong? It worked fine before the switch to GDK backend. Is it not working because Maps connects to the button-press-event twice? If so the workaround in your patch is quite fragile :) Since when a map bubble appear we will connect to that event again, to see when it should be dismissed. Should this not be fixed in clutter-gtk or in clutter? Is there some documentation that shows that Maps is misusing the API? Thanks
Or is it the grab_focus call that creates some circular thing? Is it not allowed from an event handler?
Hi Jonas, As I wrote it's an ugly workaround. I'm not sure how to actually fix this. The root of the issue is that maps is listening to a Clutter event signal and then trying to take a grab using GTK+, which generates new events, then processed by Clutter again. You can see this issue : ClutterEvent -> GtkEvent -> ClutterEvent (deadlock).
Maybe the right thing to do would be to listen to GTK+ events instead of Clutter, and just use Clutter as a rendering API.
Yeah, maybe... Not sure why these button-press-events listens to the Clutter actor instead of the GtkClutterEmbed widget, will have to investigate if there is a real reason. And we are interacting with the clutter based libchamplain map widget. So, will have to see that it works out with its events as well. I guess. Or, mandate, as with your work-around patch above, that no code that generates clutter events may be called from a clutter event handler directly.
I commited a work-around fix, sort of the one Lionel suggested, in Maps. Not sure if more needs to be done with this bug? Should it stay open?
I think there isn't much we can do here. If you could find a way to make all event handling using GTK+, that would be the sanest solution at the moment. Of course the ultimate answer would be to have something like Clutter inside GTK+ to avoid this kind of issues (GTK+4?). If you don't have time to investigate the GTK+ events, I might give it a try this week.
(In reply to Lionel Landwerlin from comment #14) > I think there isn't much we can do here. > If you could find a way to make all event handling using GTK+, that would be > the sanest solution at the moment. > > Of course the ultimate answer would be to have something like Clutter inside > GTK+ to avoid this kind of issues (GTK+4?). > > If you don't have time to investigate the GTK+ events, I might give it a try > this week. Well, libchamplain (the map widget) is clutter based, so I don't know how feasible it would be to be pure GTK+ in event handling. But it seems that the case that deadlocked is the only case of calling something that generates events from an event handler, so I hope we are good with it. I think that case also was some kind of hack to get panning with keyboard to work. Don't remember exactly.
putting this on the blocker list for tracking
So, maps seems to work just fine here (under X). Is anything left to do here, or did the workaround from comment 13 fix this for now ?
Leaving a comment following a discussion on #clutter (that I don't have any more because I lost the backtrace): A potential solution for the issue is to add a toggle inside GtkClutterEmbed that disables all event delivery from the Clutter side. GtkClutterEmbed would then consume only the windowing system events coming from GDK. This disables all event handling from the scene graph, thus making Clutter only the drawing API. The GtkWidgets embedded into the scene graph would still obviously keep working. On the other hand, if your application is using Clutter as a full scene graph, with input only on ClutterActors (for instance, you're writing a small game, or a viewer), then you'd still be safe. I can try sneaking in this API before 3.16. The situation definitely warrants a warning in the API reference of Clutter-GTK, though.
Re-assigning to clutter-gtk. I'm not sold on the API to disable event handling — mostly because it's hard to know when to use it. I've added a note to the API reference, though, so I can point people at it. I'll keep the bug open anyway, in case I change my mind, or find a decent approach.
Pushed a documentation fix as commit 464b0001e3464374e2b361caa6ef45e8584bf30e