Re: gtkdnd.c race condition?

[Owen Taylor <otaylor redhat com>]

Alexander Larsson <alla lysator liu se> writes:

> On 16 Jan 2001, Owen Taylor wrote:
> 
> > 
> > Alexander Larsson <alla lysator liu se> writes:
> > 
> > > I've just implemented drag and drop on linux-fb, and I have a problem with
> > > the generic gtkdnd code. Specifically this code in gtk_drag_update:
> > > 
> > >   if (gdk_drag_motion (info->context, dest_window, protocol,
> > > 		       x_root, y_root, action,
> > > 		       possible_actions,
> > > 		       time))
> > >     {
> > >       if (info->last_event)
> > > 	gdk_event_free ((GdkEvent *)info->last_event);
> > > 
> > >       info->last_event = gdk_event_copy ((GdkEvent *)event);
> > >     }
> > > 
> > > I get an assert in gdk_event_copy due to the fact that info->last_event ==
> > > event.
> > > 
> > > Here is what i think happens:
> > > 1) User moves mouse, leading to a call to gtk_drag_update().
> > > 2) gdk_drag_motion() sets src_context->drag_state to
> > >    GDK_DRAG_STATUS_MOTION_WAIT and sends a GDK_DRAG_MOTION event.
> > > Before the destination gets to run gdk_drag_status() to set drag_status
> > > back to GDK_DRAG_STATUS_DRAG the user moves the mouse again.
> > > 3) gdk_drag_motion() is called again, but returns TRUE because it will not
> > > send anything until it gets gdk_drag_status(). Since it returns true, the
> > > event will be stored in info->last_event.
> > > 4) now we get the GDK_DRAG_STATUS event, which leads
> > > gtk_drag_source_handle_event() calls gtk_drag_update() with the
> > > info->last_event as parameter.
> > 
> > First, I'd say that code gtkdnd.c is, at least implicitely, quite
> > specific to the X drag-and-drop protocols and if it works for you
> > doing a local drag-and-drop protocol not via X, that is at to some
> > extent coincidence. :-)
> > 
> > There is no reason why your code for GdkFB needs to use the GDK
> > DND events at all if it isn't convenient, or the same gdkdnd.h
> > interface, since these aren't public interfaces. The only public
> > interface is that in gtkdnd.h.
> 
> I must say that given my complete ignorance of how drag and drop works in
> Gtk+ i was pretty amazed when my first try at converting gdkdnd-x11.c to
> gdkdnd-fb.c worked at all. :)
> 
> But it might be better to conditionally compile gtkdnd.c to use a better
> protocol for local dnd. I might look into this when there are no other
> pressing issues.
>  
> > Anyways, yes you are right about the race condition, although
> > it will almost never happen for X.
> > 
> > The basic problem is you are running into stuff being done
> > between the point where the context->drag_state is reset
> > to GDK_DRAG_STATUS_DRAG and the point where the accompanying
> > GDK_DRAG_STATUS event is received and gdk_drag_motion()
> > is called again.
> > 
> > In the X port, these two events are tightly bound together - one
> > occurs when the event is translated into a GdkEvent prior to
> > dispatching and the second occurs when the event is actually
> > dispatched. These events, in some circumnstance, be separated,
> > but almost never are, at least by any significant amount.
> > 
> > For the FB port, these events are quite separated since the
> > first occurs immediately when gdk_drag_status() is called.
> > (There is no event translation step for the FB port.)
> > 
> > 
> > Your patch is probably fundamentally OK - it certainly is somewhat
> > cleaner if this case is handled, but I don't think, even with the
> > framebuffer port, it should ever happen in practice.
> In practice, it happens quite often. Looking at my logs, i can see it
> only ever happens after i drag from one window to
> another. gdk_drag_motion() does gdk_drag_do_leave() and then resets
> drag_state to GDK_DRAG_STATUS_DRAG, and then sends the GDK_DRAG_STATUS 
> event. Then it sends the motion event, and immediately sends the status to
> MOTION_WAIT.

What I meant was, "if the code is fixed up to be correct in
other ways (as with my patch), then the case of 'event == last_event'
should never happen"

Regards,
                                        Owen