public inbox for cygwin-xfree@sourceware.org
help / color / mirror / Atom feed
From: Michel Hummel <hummel.michel@gmail.com>
To: cygwin-xfree@cygwin.com
Subject: Re: XWin crash after the launch of startkde on a remote Red Hat 5 machine
Date: Mon, 20 Sep 2010 14:49:00 -0000	[thread overview]
Message-ID: <AANLkTikNa+9uSOddSnY6uDxkPiNifpYg2M+S2B8vm0n7@mail.gmail.com> (raw)
In-Reply-To: <4C9768C1.2050508@dronecode.org.uk>

2010/9/20 Jon TURNEY <jon.turney@dronecode.org.uk>:
> On 18/09/2010 19:44, michel hummel wrote:
>>
>> I have checked the code of the clipboard thread and the race condition
>> you talk about should not be a problem.
>> Before to start a new clipboard thread, the code waits for the end of
>> the first thread (pthread_join) then fixe the variable
>> g_fClipboardLaunched = FALSE;
>> g_fClipboardStarted = FALSE;
>>
>> So the new thread will never be launched before the old one has
>> terminated.
>>
>> I have a proposition of modification to make the clipboard thread more
>> robust.
>>
>> 1/ Adding a cleanup function which will be automaticaly called at the
>> end of the thread.
>> I purpose the use of the macro pthread_cleanup_push()
>> pthread_cleanup_pop()
>>
>> 2/ You said that having a long-lived thread will be a good solution to
>> simplify the code.
>> I have an other proposition which needs less modification and can
>> simplify the code :
>> By using also here the macro pthread_cleanup_push() we can
>> automaticaly restart the thread on every unexpected exit.
>> We just have to delete this restart when the exit is expected (ie.
>> End of the function)
>> With this solution we don't have to take care about the thread
>> being killed by anyone, it will be restarted.
>
> You will need to have some kind of back-off mechanism (i.e. a delay before
> retrying to connect) so that that this doesn't constantly try to connect if
> the server gets into a state where it can't accept the connection or the
> connection is constantly getting closed.

Like I’ve implemented it :
- The clipboard thread uses the winProcEstablishConnection wrapper to
restart and so it’s waiting for a new client connection before trying
to reconnect himself.
- An other point, the restart mechanism will only be activated if the
clipboard thread has successfully been connected, otherwise the
clipboard thread will dies and not restarts (this will prevent an
infinite loop on connection error)
Those two mechanisms and the loop on  XOpenDisplay with the sleep
(WIN_CONNECT_DELAY) between each retries should do the job isn't it ?
But if you think a connection delay is also needed in the restart
process can add one without problem

>
>> 3/ An other thing I saw is that when the Xwindow part of the clipboard
>> is killed by an other application, the thread will still be alive but
>> the clipboard doesn't work anymore.
>> A solution can be :
>> when an winClipboardErrorHandler() is catched, check for
>> this Xwindow window and whenever this window doesn't exist anymore,
>> create a new one.
>
> Would it not be simpler to restart the clipboard thread in this situation as
> well, rather than having code to recrate the window?

You are absolutely right, I just have to replace the window creation
with a pthread_exit(NULL);


>
>> What do you think about my suggestions ?
>>
>> I tested them and they seem to work as expected.
>> For example, I skipped the wrapper of XQueryTree (actualy needed for
>> xdmcp) and the clipboard is now working with xdmcp/gdm, etc.
>>
>> If you don't like something in my propositions, please tell me what
>> and I will try to adapt it.
>>
>> If you are interested in a patch doing this, I will be happy to give
>> it after a quick review and a clean
>
> That would be great, thank you.
>
> --
> Jon TURNEY
> Volunteer Cygwin/X X Server maintainer
>

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


  reply	other threads:[~2010-09-20 14:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <AANLkTikJdnM+_7z5qvrdyannbTUifMGodisQniJd6-GW@mail.gmail.com>
2010-08-03  7:55 ` Michel Hummel
2010-08-06 12:55 ` Michel Hummel
2010-08-08 13:14   ` Jon TURNEY
2010-09-18 18:44     ` michel hummel
2010-09-20 13:59       ` Jon TURNEY
2010-09-20 14:49         ` Michel Hummel [this message]
2010-09-22  9:03           ` Michel Hummel
2010-09-22 14:30             ` Jon TURNEY
2010-09-22 17:11               ` XWin.exe parms wrapped by startwin.exe Fouts, Christopher
2010-09-23 12:39                 ` Jon TURNEY
2010-09-24  8:15               ` XWin crash after the launch of startkde on a remote Red Hat 5 machine Michel Hummel
2010-09-24 15:04                 ` Michel Hummel
2010-09-28 20:12                 ` Jon TURNEY
2010-09-28 21:28                   ` Jim Reisert AD1C
2010-09-29  8:59                   ` Michel Hummel
2010-09-29 14:05                     ` Michel Hummel
2010-10-31 16:59                       ` Jon TURNEY

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTikNa+9uSOddSnY6uDxkPiNifpYg2M+S2B8vm0n7@mail.gmail.com \
    --to=hummel.michel@gmail.com \
    --cc=cygwin-xfree@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).