Keith Packard escreveu:
The input thread will block before enqueuing while the X server pulls
events out of the queue, causing it to stop reading events and updating
the cursor.

Indeed.

The following patch assert the order of events enqueued with a mutex protecting writes to the tail pointer in mieqEnqueue. If the main thread blocks somehow, the input thread continues doing its job. All mieqEnqueue users must be safe now.


The X server also reads the input queue pointers from DIX before calling
into ProcessInputEvents, so you aren't safe against memory write races
(where writes to one location are visible across CPUs before writes to
another location).

I think this can be easily changed by nonblocking pipes. If someone wants to wakeup PIE, then just write on the pipe in which this function is listening. I have to think a little more about this but it may be easy to address.


So, we either need to fix the users of checkForInput along with all
users of miEventQueue, or we need to assert that we're running on a
sensible CPU that doesn't require a mutex to make memory changes visible
across processors, in which case we don't need any mutex at all.

I really don't know how to check if a CPU is visible to the other CPU.


Thanks,

--
Tiago Vignatti
C3SL - Centro de Computação Científica e Software Livre
www.c3sl.ufpr.br
diff --git a/mi/mieq.c b/mi/mieq.c
index 0a1b740..d96c545 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -53,6 +53,10 @@ in this Software without prior written authorization from The Open Group.
 # include   "extinit.h"
 # include   "exglobals.h"
 
+#ifdef INPUT_THREAD
+#include   <pthread.h>
+#endif
+
 #ifdef DPMSExtension
 # include "dpmsproc.h"
 # define DPMS_SERVER
@@ -81,6 +85,10 @@ typedef struct _EventQueue {
 
 static EventQueueRec miEventQueue;
 
+#ifdef INPUT_THREAD
+pthread_mutex_t miTailQueueMutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
 Bool
 mieqInit(void)
 {
@@ -122,6 +130,9 @@ mieqResizeEvents(int min_size)
 void
 mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
 {
+#ifdef INPUT_THREAD
+    pthread_mutex_lock(&miTailQueueMutex);
+#endif
     unsigned int           oldtail = miEventQueue.tail, newtail;
     EventListPtr           evt;
     int                    isMotion = 0;
@@ -146,6 +157,9 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
 
         if (laste->nevents > 6) {
             ErrorF("[mi] mieqEnqueue: more than six valuator events; dropping.\n");
+#ifdef INPUT_THREAD
+            pthread_mutex_unlock(&miTailQueueMutex);
+#endif
             return;
         }
         if (oldtail == miEventQueue.head ||
@@ -157,10 +171,16 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
             ((lastkbp->deviceid & DEVICE_BITS) !=
              (v->deviceid & DEVICE_BITS))) {
             ErrorF("[mi] mieqEnequeue: out-of-order valuator event; dropping.\n");
+#ifdef INPUT_THREAD
+            pthread_mutex_unlock(&miTailQueueMutex);
+#endif
             return;
         }
 
         memcpy((laste->events[laste->nevents++].event), e, sizeof(xEvent));
+#ifdef INPUT_THREAD
+        pthread_mutex_unlock(&miTailQueueMutex);
+#endif
         return;
     }
 
@@ -176,6 +196,9 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
 	if (newtail == miEventQueue.head) {
             ErrorF("[mi] EQ overflowing. The server is probably stuck "
                    "in an infinite loop.\n");
+#ifdef INPUT_THREAD
+        pthread_mutex_unlock(&miTailQueueMutex);
+#endif
 	    return;
         }
 	miEventQueue.tail = newtail;
@@ -193,6 +216,9 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
         if (!evt->event)
         {
             ErrorF("[mi] Running out of memory. Tossing event.\n");
+#ifdef INPUT_THREAD
+            pthread_mutex_unlock(&miTailQueueMutex);
+#endif
             return;
         }
     }
@@ -212,6 +238,9 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
     miEventQueue.events[oldtail].pDev = pDev;
 
     miEventQueue.lastMotion = isMotion;
+#ifdef INPUT_THREAD
+    pthread_mutex_unlock(&miTailQueueMutex);
+#endif
 }
 
 void
_______________________________________________
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg

Reply via email to