This is the mail archive of the cygwin-xfree mailing list for the Cygwin XFree86 project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: checkX problems

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



Unsubscribe info:
Problem reports:

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]