X11 event reordering causes problems

[Jukka Heinonen]

In some cases, Wine handles X11 events from multiple threads.
Since event handling has not been serialized, it is possible
that X11 events are handled in different order than they were
present in the event queue. 

This causes a repeatable problem when keyboard is repeating:
in this case, X11 sends KeyPressed,KeyReleased event pairs
and every now and then the order of this pair gets reversed
into KeyReleased,KeyPressed. If this pair is the last occurring,
Wine incorrectly believes that a released key is pressed and
GetAsyncKeyState returns incorrect values. Reordered keyboard 
events are also likely to cause problems in quite a few other 
places.

In practice, the above reordering occurs rarely because 
when I play games, I don't often keep keys pressed so long 
that repeating starts. However, this is very annoying when
it does occur and it is possible that the same problem
occurs when you quickly tap a key (less likely because
there will likely be a 10-100 millisecond delay between
KeyPress and KeyRelease in this case).

Quick solution (shown in the patch below) is that keyboard 
events are handled only from a single thread. Even though this 
means that other events might be processed before keyboard events, 
this should not cause any real problems.

However, I am a bit worried what happens when other events get reordered
as mentioned above. Mouse event reordering is not a problem and might
be solved in the same way. But what happens if for example 
FocusIn and FocusOut events get swapped? And there might be event combinations
the swapping of which causes Wine to crash or work incorrectly.

Log:
Jukka Heinonen <jhei at iki.fi>
Prevented keyboard events from being reordered even though
X11 events are processed from multiple threads.

Index: event.c
===================================================================
RCS file: /home/wine/wine/windows/x11drv/event.c,v
retrieving revision 1.96
diff -u -r1.96 event.c
--- event.c     2001/05/09 19:48:32     1.96
+++ event.c     2001/05/15 07:59:48
@@ -181,18 +181,20 @@
 static void CALLBACK EVENT_ProcessAllEvents( ULONG_PTR arg )
 {
     XEvent event;
-
-    TRACE( "called (thread %lx).\n", GetCurrentThreadId() );
+    long  *mask = (long *)arg;
 
-    wine_tsx11_lock();
-    while ( XPending( display ) )
-    {
-        XNextEvent( display, &event );
-        wine_tsx11_unlock();
-        EVENT_ProcessEvent( &event );
-        wine_tsx11_lock();
-    }
-    wine_tsx11_unlock();
+    TRACE( "called (thread %lx), event mask is %lx.\n",
+          GetCurrentThreadId(),
+          mask ? *mask : ~None );
+
+    wine_tsx11_lock();
+    while ( XCheckMaskEvent( display, mask ? *mask : ~None, &event ) )
+    {
+      wine_tsx11_unlock();
+      EVENT_ProcessEvent( &event );
+      wine_tsx11_lock();
+    }
+    wine_tsx11_unlock();
 }
 
 /***********************************************************************
@@ -202,8 +204,14 @@
  */
 void X11DRV_Synchronize( void )
 {
+    /*
+     * This mask value prevents reordering of key events.
+     * TODO: Event mask should only include those events that really
+     *       need to be processed asynchronously.
+     */
+    long mask = ~KeyPressMask & ~KeyReleaseMask;
     TSXSync( display, False );
-    EVENT_ProcessAllEvents( 0 );
+    EVENT_ProcessAllEvents( (ULONG_PTR)&mask );
 }                                                        

 /***********************************************************************      

-- 
Jukka Heinonen <http://www.iki.fi/jhei/>