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 623865 - gtkdnd: pointer grab may never finish (ungrab before grab)
gtkdnd: pointer grab may never finish (ungrab before grab)
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.20.x
Other Linux
: Normal major
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: motion-event-tracker 623213
 
 
Reported: 2010-07-08 17:35 UTC by Stanislav Brabec
Modified: 2010-07-23 16:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdb backtrace when grab changes (11.12 KB, application/octet-stream)
2010-07-08 17:35 UTC, Stanislav Brabec
  Details
gtk-dnd-grab-deadlock-fix.patch (828 bytes, patch)
2010-07-09 14:31 UTC, Stanislav Brabec
none Details | Review
proposed fix for gtk+-2.20.x (1.26 KB, patch)
2010-07-21 15:11 UTC, Stanislav Brabec
none Details | Review
proposed fix for head (1.20 KB, patch)
2010-07-21 15:12 UTC, Stanislav Brabec
none Details | Review

Description Stanislav Brabec 2010-07-08 17:35:14 UTC
Created attachment 165497 [details]
gdb backtrace when grab changes

How to reproduce:

1. Run epiphahy
2. Fire gdb on a different machine and attach to epiphany
3. Set breakpoints to gdk_pointer_grab and gdk_display_pointer_ungrab
4. Continue
5. Drag any image or link, shortly move and drop
6. Go to gdb, you will experience three breakpoints: one early in gtk_drag_begin_internal, second lately in gtk_drag_begin_internal and the last one in gtk_drag_button_release_cb (see attached backtrace)

Expected results:
gtk_drag_button_release_cb() releases the grab by the gdk_display_pointer_ungrab() call.

Actual results:
It apparently does not happen. You still experience mouse grab and there is possibly no way to leave it.

Note:
The problem appears even outside gdb, especially on slow machines.

Here is an excerpt of the webkit code:
GdkEvent* event = gdk_event_new(GDK_BUTTON_PRESS);
event->window = gtk_widget_get_window(GTK_WIDGET(m_webView));
event->time = GDK_CURRENT_TIME;
GdkDragContext* context = gtk_drag_begin(GTK_WIDGET(m_webView), targetList, dragAction, 1, event);

Such code may look safe and correct. But please see the GDK_CURRENT_TIME. GDK_CURRENT_TIME is evaluated in time of event processing to the current time.

On the opposite, gtk_drag_button_release_cb() is called with a real time of the event.

As there was a delay, time of processing of gdk_pointer_grab() is later than time when gdk_display_pointer_ungrab() happened. It means that grab is activated after ungrab.

Also reported as: bug 623213 (epiphany)
https://bugs.webkit.org/show_bug.cgi?id=41282 (webkit)

Reproduced on:
openSUSE 11.3 RC2 x86_64, gtk2-2.20.1, epiphany-2.30.2 libwebkit-1.2.0 (user agent
531.2)
Comment 1 Stanislav Brabec 2010-07-09 14:31:32 UTC
Created attachment 165559 [details] [review]
gtk-dnd-grab-deadlock-fix.patch

Proposed fix.

If gtk_drag_begin() uses GDK_CURRENT_TIME, then gtk_drag_end() must use GDK_CURRENT_TIME as well. Application is responsible for a correct order of events.

In all other cases, time is generated by event and GTK+ is responsible for correct order of events.

I was thinking about a situation, when one DnD happens and all events are buffered and another DnD starts. If it is handled by GTK+, grab nature and event time guarantee the correct order. If the event is handled by an artificial event from an application (e. g. webkit), webkit must make sure, that new DnD event is not created before the old finishes. I guess it should be true in most applications as well.

Time disparity should never happen now.
Comment 2 Stanislav Brabec 2010-07-21 15:11:15 UTC
Created attachment 166292 [details] [review]
proposed fix for gtk+-2.20.x

Better fix proposed by Federico.

If gtk_drag_begin() uses GDK_CURRENT_TIME in the event, try to evaluate the real time by gtk_get_current_event_time(). If it fails or if gtk_drag_begin() was called with event == NULL, then gtk_drag_end() must use GDK_CURRENT_TIME as well. Application is responsible for a correct order of events in this case.

Nothing changes in all other cases: Time generated by event is paseed to gtk_drag_begin() and processed. gtk_drag_end() uses real event time.

Note: In case of the reported webkit freeze, gtk_get_current_event_time() succeeds and returns processed event time. Freeze did not happen in my test.
Comment 3 Stanislav Brabec 2010-07-21 15:12:14 UTC
Created attachment 166293 [details] [review]
proposed fix for head
Comment 4 Federico Mena Quintero 2010-07-22 21:26:57 UTC
Pushed to master as ccc3d2c6 as and to gtk-2-22 as 13dba0a4.  Thanks, Stanislav!
Comment 5 Stanislav Brabec 2010-07-23 10:07:48 UTC
Thinking about it again, maybe we should add gtk_get_current_event_time() to else branch of if (event) as well.
Comment 6 Federico Mena Quintero 2010-07-23 16:45:11 UTC
... or we could move the "if (time == GDK_CURRENT_TIME)" outside the "if (event)" block for the same effect.

I don't know if that would be helpful for anything... if you are calling gtk_drag_begin() with a NULL event, then you are either doing something wrong (you *had* an event and you are not passing it), or you are starting a drag out of nowhere (kind of evil in UI terms).