Re: xquartz dereferencing a NULL pointer (patch 2)

[Jeremy Huddleston]

On Nov 5, 2008, at 11:37, George Peter Staplin wrote:

On Nov 5, 2008, at 10:35 AM, Jeremy Huddleston wrote:

I see two possibilities for handling this:

1) Lock inside mieqProcessInputEvents

I think the pthread locking is actually safe in xquartz. I can't find any places calling mieqEnqueue from a signal handler, so that comment I made earlier about signals and deadlocks probably doesn't apply to XQuartz.

2) Disable this optimization when we have multiple threads playing with mieqEventQueue and just push a second MotionNotify event.

Hmm, I don't fully understand that.  Can you explain more?

diff --git a/mi/mieq.c b/mi/mieq.c
index e93d24f..a634bf8 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -235,6 +236,10 @@ mieqProcessInputEvents(void)
     int x = 0, y = 0;
     DeviceIntPtr dev = NULL;

+#ifdef INPUT_THREAD
+    pthread_mutex_lock(&miEventQueueMutex);
+#endif
+
     while (miEventQueue.head != miEventQueue.tail) {
         if (screenIsSaved == SCREEN_SAVER_ON)
             SaveScreens (SCREEN_SAVER_OFF, ScreenSaverReset);
@@ -249,6 +254,10 @@ mieqProcessInputEvents(void)

         miEventQueue.head = (miEventQueue.head + 1) % QUEUE_SIZE;

+#ifdef INPUT_THREAD
+        pthread_mutex_unlock(&miEventQueueMutex);
+#endif
+
         if (miEventQueue.handlers[e.event[0].u.u.type]) {

              * steal it. */
@@ -283,5 +292,14 @@ mieqProcessInputEvents(void)

             dev->public.processInputProc(e.event, dev, e.nevents);
         }
+
+#ifdef INPUT_THREAD
+        pthread_mutex_lock(&miEventQueueMutex);
+#endif
     }
+
+#ifdef INPUT_THREAD
+    pthread_mutex_unlock(&miEventQueueMutex);
+#endif
+
 }





diff --git a/mi/mieq.c b/mi/mieq.c
index 062dede..25136b2 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -122,7 +122,8 @@ mieqResizeEvents(int min_size)
 void
 mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
 {
-    unsigned int           oldtail = miEventQueue.tail, newtail;
+    unsigned int           oldtail = miEventQueue.tail;
+    unsigned int           newtail = oldtail;
     EventListPtr           evt;
     int                    isMotion = 0;
     int                    evlen;
@@ -164,11 +165,16 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
         return;
     }

+#ifndef INPUT_THREAD
     if (isMotion && isMotion == miEventQueue.lastMotion &&
         oldtail != miEventQueue.head) {
        oldtail = (oldtail - 1) % QUEUE_SIZE;
+
+       miEventQueue.tail = oldtail;
     }
-    else {
+    else
+#endif
+    {
        static int stuck = 0;
        newtail = (oldtail + 1) % QUEUE_SIZE;
        /* Toss events which come in late.  Usually this means your server's
@@ -184,7 +190,6 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
            return;
         }
        stuck = 0;
-       miEventQueue.tail = newtail;
     }

     evlen = sizeof(xEvent);
@@ -218,6 +223,7 @@ mieqEnqueue(DeviceIntPtr pDev, xEvent *e)
     miEventQueue.events[oldtail].pDev = pDev;

     miEventQueue.lastMotion = isMotion;
+    miEventQueue.tail = newtail;
 }

 void

1) Lock inside mieqProcessInputEvents (this is based off of xorg- server-1.4-apple... but the concept applies to master after Peter pushes the changes discussed over the past couple days) memcpy(&e, &miEventQueue.events[miEventQueue.head], sizeof(EventRec)); /* If someone's registered a custom event handler, let them 2) Avoid the "replace the last motion-event with this one" optimization while using an input thread. I think this is ugly, but I'm mentioning it for completeness.

smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg