Re: checkX problems

[Charles Wilson]

Lothar Brendel wrote:
> Charles Wilson already set up this kind of infrastructure, I just had to
> introduce one more communication variable, cf. the patch below
> (positively tested on my system).
> 
> Yep, there are really two different purposes for a setting a timeout [i)
> "Just check whether an X server is available, but don't struggle with
> that too long." and ii) "There *should* be an X server coming up, just
> be a little patient."], but now both can be achieved by choosing either
> a short or a long duration.

Looks pretty good.  There's still one problematic case -- but it
actually already existed; your change doesn't make it any worse than
what was already there.

> +  do
> +if((dpy = (*(data->xopendis))(data->displayname))) {

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.  Instead, it destroys it while still having it
locked.  Then, it evaluates xopenOK to compute the return value.  The
spec says:

"It shall be safe to destroy an initialized mutex that is unlocked.
Attempting to destroy a locked mutex results in undefined behavior."

So, the child thread might be stuck waiting for a mutex that has already
been destroyed.  That could be a problem -- but a very very rare one, I
think.  It only happens if you time out on the worker -- and THEN,
before the main app gets to exit(), the worker successfully returns from
XOpenDisplay. (If the main thread exits(), that should kill the worker
thread...so it never gets a chance to return successfully or otherwise).

> +  xopenOK = XSERV_FOUND;
> +  pthread_cond_signal(&cv_xopenOK);
> +  pthread_mutex_unlock (&mtx_xopenOK);
> +}
> +  while (xopenTrying && xopenOK == XSERV_NOTFOUND);
> 
>   pthread_exit((void*)0);
> }

However, (a) it's working now, even if it is technically "wrong" to do
it that way, and (b) it gets real complicated to figure out how to
guarantee the mutex is unlocked, in both threads, before destroying it
-- without forcing the calling thread to join() the worker, which is
explicitly what we DON'T want to do in this case.

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

Thanks!

--
Chuck


--
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/

Re: checkX problems

[Lothar Brendel]

Mike Ayers wrote:

From: cygwin-xfree-ow...@cygwin.com [mailto:cygwin-xfree-
ow...@cygwin.com] On Behalf Of Lothar Brendel



Could you please clarify an issue here? (Sorry, it seems, I wronged
to ``run'' in the previous posts.)

In a Windows command prompt (being somewhere on C:) I put the line
\cygwin\bin\run -p /usr/bin sleep -wait 5
into a file ``dosleep.bat''. Executing that BAT-script (w/o any
wrapper), it
*does* sleep. Typing that very line directly at the prompt lets
``run'' return immediately, though. Can you confirm this behaviour?


I can confirm that without testing (so I'm probably chomping foot
here...).  The sleep is holding the console open after run quits.


Sorry, I don't quite get it. *Which* console must it hold open? There is 
only the one I'm typing in and calling the BAT-script doesn't open another 
one. Hence, in how far does the environment of the BAT-script differ from 
the one of the command prompt?




Another possibility would be an option ``-n'' to specify the number
of retries.


GAH!  No, that's just lame.


All right, all right, you just convinced me. :-)



Just spawn/fork a
sleep-then-interrupt-daddy thread/process, set up a SIGINT handler
that exits with an error, loop connection attempts until successful,
check X, kill child, exit with success.  That enforces both types of
timeout.


Charles Wilson already set up this kind of infrastructure, I just had to 
introduce one more communication variable, cf. the patch below (positively 
tested on my system).


Yep, there are really two different purposes for a setting a timeout [i) 
"Just check whether an X server is available, but don't struggle with that 
too long." and ii) "There *should* be an X server coming up, just be a 
little patient."], but now both can be achieved by choosing either a short 
or a long duration.


Ciao
   Lothar


--- checkX.c-0.3.0 2009-06-15 02:29:07.0 +0200
+++ checkX.c 2009-11-14 19:36:31.0 +0100
@@ -32,6 +32,7 @@
#endif

#include 
+#include 

#if HAVE_SYS_TYPES_H
# include 
@@ -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,14 @@
  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 mutex */
+  xopenTrying = 0;  /* allow open_display() to give up */
+}/* else open_display() was successful */
+pthread_detach(id);  /* leave open_display() 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 +361,17 @@
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);
+  pthread_mutex_lock (&mtx_xopenOK);
+  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/