Re: checkX problems

["Lothar Brendel" <lottas dot junk at geekmail dot de>]

Charles Wilson wrote:

[...]

> The call to XOpenDisplay can take up to 12 seconds. Suppose the main
> thread times out after say 5 seconds, and then just after that we
> have a *successful* return in the worker thread.  The worker thread
> tries to get the mutex:
>
> > +      (*(data->xclosedis))(dpy);
> > +      pthread_mutex_lock (&mtx_xopenOK);
>
> But the main thread, if you follow the timed-out codepath, never
> releases the mutex.

Ok, this can be cured by

   if (pthread_cond_timedwait (&cv_xopenOK, &mtx_xopenOK, &then) == 
ETIMEDOUT) {
     xopenOK = XSERV_TIMEDOUT; /* it's okay, we have the mutex */
     xopenTrying = 0;  /* allow open_display() to give up */
+      pthread_mutex_unlock(&mtx_xopenOK); /* allow for a last minute change 
*/
   }    /* else open_display() was successful */
   pthread_detach(id);  /* leave open_display() on its own */

But the problem is not so much destroying a locked mutex, but rather locking 
a destroyed mutex, right?
This happens in your race condition but also whenever ``delay'' is shorter 
than the time spent in a successful XOpenDisplay(). The failure doesn't 
really harm, but we can be less dirty by checking the result of 
pthread_mutex_unlock(), cf. the new patch.

[...]

> So, I'm just going to leave it, and take your patch as-is.

Maybe you consider the new one instead.


> Thanks!

My pleasure!

Ciao
           Lothar

--- checkX.c-0.3.0 2009-06-15 02:29:07.000000000 +0200
+++ checkX.c 2009-11-15 12:32:24.000000000 +0100
@@ -32,6 +32,7 @@
#endif

#include <stdio.h>
+#include <errno.h>

#if HAVE_SYS_TYPES_H
# include <sys/types.h>
@@ -102,7 +103,8 @@

static pthread_mutex_t mtx_xopenOK;
static pthread_cond_t  cv_xopenOK;
-static int xopenOK = XSERV_TIMEDOUT;
+static int xopenOK;
+static int xopenTrying;
static const char* XLIBfmt = "cygX11-%d.dll";
static const char* DefaultAppendPath = "/usr/X11R6/bin" SEP_CHAR 
"/usr/bin";

@@ -314,6 +316,9 @@
  timespec_t     delta;
  timespec_t     then;

+  xopenTrying = delay!=0.0; /* false actually means: try once */
+  xopenOK = XSERV_NOTFOUND; /* a pessimistic start out */
+
  computeTimespec(fabs(delay), &delta);
  debugMsg(1, "(%s) Using delay of %d secs, %ld nanosecs (%5.2f)", 
__func__,
           delta.tv_sec, delta.tv_nsec,
@@ -333,15 +338,15 @@
  if (delay != 0.0) {
    clock_gettime(CLOCK_REALTIME, &now);
    timerspec_add(&now, &delta, &then);
-    pthread_cond_timedwait (&cv_xopenOK, &mtx_xopenOK, &then);
-  }
-
-  pthread_mutex_unlock(&mtx_xopenOK);
-
-  if (delay != 0.0) {
-    pthread_detach(id);
+    if (pthread_cond_timedwait (&cv_xopenOK, &mtx_xopenOK, &then) == 
ETIMEDOUT) {
+      xopenOK = XSERV_TIMEDOUT; /* it's okay, we have the lock */
+      xopenTrying = 0;  /* allow open_display() to give up */
+      pthread_mutex_unlock(&mtx_xopenOK); /* but also allow for a last 
minute change */
+    }    /* else open_display() was successful */
+    pthread_detach(id);  /* leave it on its own */
  } else {
-    pthread_join(id, (void*)&status);
+    pthread_mutex_unlock(&mtx_xopenOK); /* allow open_display() to set 
xopenOK */
+    pthread_join(id, (void*)&status); /* and wait for it */
  }

  pthread_mutex_destroy(&mtx_xopenOK);
@@ -357,19 +362,20 @@
open_display(void* /* WorkerThreadData* */ v)
{
  Display* dpy;
-  int rc = 0;
  WorkerThreadData* data = (WorkerThreadData*)v;

-  if( (dpy = (*(data->xopendis))(data->displayname)) == NULL ) {
-    rc = 1;
-  } else {
-    (*(data->xclosedis))(dpy);
-    rc = 0;
-  }
-  pthread_mutex_lock (&mtx_xopenOK);
-  xopenOK = rc;
-  pthread_cond_signal(&cv_xopenOK);
-  pthread_mutex_unlock (&mtx_xopenOK);
+  do
+    if((dpy = (*(data->xopendis))(data->displayname))) {
+      (*(data->xclosedis))(dpy);
+      if (pthread_mutex_lock (&mtx_xopenOK)) /* the mutex may already be 
destroyed */
+ break;
+      else {
+ xopenOK = XSERV_FOUND;
+ pthread_cond_signal(&cv_xopenOK);
+ pthread_mutex_unlock (&mtx_xopenOK);
+      }
+    }
+  while (xopenTrying && xopenOK == XSERV_NOTFOUND);

  pthread_exit((void*)0);
}


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/