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

Sun Aug 8 13:14:00 GMT 2010

On 06/08/2010 13:55, Michel Hummel wrote:
 > I didn't saw any response to my patch proposition, did someone tried it ?
 > If I didn't posted it in the good place where should I have to

Yes, this is abolutely the right place for your patch.

It looks good and I shall incorporate it in the next X server release.

 > I will be happy to explain the problem if my first mail wasn't clear.

Nope, the analysis was very clear, thank you.

I wonder if there also exists a race condition when the clipboard thread is 
stopping? i.e. if we try to start a new one just as the old one is stopping, 
we think it is still running and fail to do so?

In general this code could do with a tidy up: I think it would be much more 
sensible to have a long-lived thread which tries to reconnect to the server 
when ever it gets disconnected, it would avoid all this complexity and the 
complexity of trying to avoid being killed by GDM as well.

> 2010/8/3 Michel Hummel<hummel.michel@gmail.com>:
>> Hello,
>> I'm using cygwin on Microsoft Windows XP with XWin version :
>> sh-3.2# XWin.exe -version
>> Welcome to the XWin X Server
>> Vendor: The Cygwin/X Project
>> Release: 1.8.0.0 (10800000)
>> Build Date: 2010-04-02
>> Contact: cygwin-xfree@cygwin.com
>>
>> I think I have found a bug in the XWin reset procedure which leads to
>> the start of two clipboard thread's and sometime to a crash of the X
>> server.
>>
>> The crash can be produced by lauching startkde on a Red Hat 5 remote
>> machine (The crash should arrive after some minutes of work)
>> but the bug can simply be produced like this :
>>
>> On my installation if I launch Xwin with this options :
>> XWin :0 -ac&
>> then I do
>> xsetroot -solid '#FFFF00'
>>
>> After setting the color, Xwin launches its reset procedure because
>> there is no more client connected ( the server loses the color but it
>> is the normal comportment isn't it ?
>> http://sourceware.org/ml/cygwin-xfree/2002-01/msg00106.html)
>>
>> The problem comes from the clipboard thread. In this example, when the
>> server launches its reset procedure, the clipboard is in state
>> "Launched" but is not in state "Started" (Boolean variable about
>> clipboard status, see file hw/xwin/winclipboardthread.c).
>>
>> This particular status of the ClipboardThread during server reset is
>> normaly managed by two "if" statements :
>>
>> * An "if" in the file hw/xwin/InitOutput.c disables the exit of the
>> clipboard thread in this state
>> Extract of the file hw/xwin/InitOutput.c (with line number) :
>>
>>   169 winClipboardShutdown (void)
>>   170 {
>>   171   /* Close down clipboard resources */
>>   172   if (g_fClipboard&&  g_fClipboardLaunched&&  g_fClipboardStarted)
>>   173     {
>>   174       /* Synchronously destroy the clipboard window */
>>   175       if (g_hwndClipboard != NULL)
>>   176         {
>>   177           SendMessage (g_hwndClipboard, WM_DESTROY, 0, 0);
>>   178           /* NOTE: g_hwndClipboard is set to NULL in
>> winclipboardthread.c */
>>   179         }
>>   180       else
>>   181         return;
>>   182
>>   183       /* Wait for the clipboard thread to exit */
>>   184       pthread_join (g_ptClipboardProc, NULL);
>>   185
>>   186       g_fClipboardLaunched = FALSE;
>>   187       g_fClipboardStarted = FALSE;
>>   188
>>   189       winDebug ("winClipboardShutdown - Clipboard thread has exited.\n");
>>   190     }
>>   191 }
>>
>> * An "if" statement in the file hw/xwin/winclipboardwrappers.c
>> prohibits the launch of clipboard thread if one is already Launched.
>> Extract of the file hw/xwin/winclipboardwrappers.c (with line number)
>> 256   /* If the clipboard client has already been started, abort */
>> 257   if (g_fClipboardLaunched)
>> 258     {
>> 259       ErrorF ("winProcEstablishConnection - Clipboard client already "
>> 260               "launched, returning.\n");
>> 261       return iReturn;
>> 262     }
>>
>>
>> The problem is that the Boolean variables g_fClipboardLaunched and
>> g_fClipboardStarted are re-initialized by the server reset procedure
>> (function winInitializeGlobals of the file  hw/xwin/winglobals.c)
>> Extract of the file hw/xwin/winglobals.c (with line number)
>> 128 /*
>> 129  * Re-initialize global variables that are invalidated 130  * by a
>> server reset.
>> 131  */
>> 132
>> 133 void
>> 134 winInitializeGlobals (void)
>> 135 {
>> 136   g_dwCurrentThreadID = GetCurrentThreadId ();
>> 137   g_hwndKeyboardFocus = NULL;
>> 138 #ifdef XWIN_CLIPBOARD
>> 139   g_fClipboardLaunched = FALSE;
>> 140   g_fClipboardStarted = FALSE;
>> 141   g_iClipboardWindow = None;
>> 142   g_pClipboardDisplay = NULL;
>> 143   g_atomLastOwnedSelection = None;
>> 144   g_hwndClipboard = NULL;
>> 145 #endif
>> 146 }
>>
>> The consequence of this Re-initialization in this particular situation
>> is that the clipboard thread is launched two times and sometimes leads
>> to a crash of the X server.
>> You can see the double launch of the clipboard thread at the end of
>> the attached log file Xwin.0.log ( 2 times the sentence :
>> winClipboardProc - XOpenDisplay () returned and successfully opened
>> the display. )
>>
>> To fix this bug I purpose to remove the variables g_fClipboardLaunched
>> and g_fClipboardStarted of the "winInitializeGlobals (void)" function,
>> as their re-initializations are handled in in the files :
>> hw/xwin/InitOutput.c.
>>
>> That what is doing the patch Attached to this email.

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