Re: XWin crash after the launch of startkde on a remote Red Hat 5 machine

[Michel Hummel <hummel dot michel at gmail dot com>]

Hello,
I've modified my patch, to change the restart process.
It does not use anymore the winProcEstablishConnection wrapper to
restart the clipboard but directly the winInitClipboard function.

This allows to restart the clipboard more quickly and if the clipboard
thread cannot connects to the server (there is a loop on connection
with a delai between retries), it will die.

One question :
I have written my patch for the git version of the X server and the
patch is not working as it on the 1.8.0-1 version.
Which version would you like,  perhaps the two ?

Michel Hummel

2010/9/20 Michel Hummel <hummel.michel@gmail.com>:
> 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/