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

[Michel Hummel <hummel.michel@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 22409 bytes --]

You will find attached to this Email the modifications you asked.

The X server doesn't freeze anymore when clipboard restarts on xdmcp,
certainly fixed by the cygwin patches.

I added a rate-limit of clipboard_generation which disables restart after
XWIN_CLIPBOARD_RETRIES retries (defined to 40 in winclipboard.h)
and a sleep(XWIN_CLIPBOARD_DELAY) before retries (defined to 1 in
winclipboard.h)

I hope you will like this new version,

Thank you
Michel Hummel

2010/9/29 Michel Hummel <hummel.michel@gmail.com>:
> Thank you for your comments, I really want to help you on this part of
> the X server.
>  I will now do :
> 1/ Adapt the patch to be cygwin Xorg compatible
> 1/ Split the patch into two separate patches
> 2/ Publish as soon as possible a new patches version which will take
> into account your comments :
> *a) WM_DESTROYCLIPBOARD message (Could you be a little more precise
> concerning this, I'm not sure to realy understand the issue
> *b) Adding a rate-limit on "clipboard_generation" with a static
> variable for example ? and a restart delay (500 Milliseconds should be
> enough isn't it ?)
> *c) Change the comment about ProcEstablishConnection wrapper and inInitClipboard
> *d) Merge the cleanup and the restart function into one
> *e) Concerning the use of pthread_cleanup_pop at winClipboardProc end,
> do you mean : Replace all pthread_exit() call's by a goto error_exit
> label which will call pthread_cleanup_pop(1) (The parameter "1" means
> that we have to execute it)
> *f) Delete the unnecessary whitespace changes
> *g) Concerning the winClipboardErrorHandler tricks, This is for the
> situation you said : "Window has been deleted by someone else, but we
> are still connected to the server" I will add a comment in the sources
>
> Michel Hummel
>
>
> 2010/9/28 Jon TURNEY <jon.turney@dronecode.org.uk>:
>> On 24/09/2010 09:15, Michel Hummel wrote:
>>>
>>> Hi Jon,
>>
>> Firstly, thanks very much for looking at this.
>>
>>> You will find attached to this Email a patch (for the git version)
>>> which implements for the clipboard thread :
>>>  - Auto cleanup function
>>>  - Auto restart function for unexpected exit of the clipboard or
>>> Xerror detection
>>>  - Deletion of XQuery wrapper  (not needed anymore)
>>>  - Deletion of all the xdmcp related tricks  (not needed anymore)
>>
>> For purposes of review it would be easier to split this patch into two
>> separate patches, (i) the clipboard thread changes and (ii) removing the
>> unneeded xdmcp tricks.
>>
>>> One thing, this patch deletes the call to EmptyClipboard () in the
>>> winProcSetSelectionOwner function of the winclipboardwrappers.c file
>>> when an "owned to not owned transition" has been detected. I don't
>>> know why but it was freezing the server for 1 or 2 seconds during
>>> clipboard's restart in xdmcp connection.
>>
>> Hmmm... I wonder if that's something to do with how we process the
>> WM_DESTROYCLIPBOARD message?  In any case, this should not be happening.
>>
>>> It would be fine if you could tell me, when you think this patch and
>>> the previous one ( which makes the reset of the server working
>>> correctly with clipboard) will be included to the official X server?
>>
>> We don't have a regular release cycle for the cygwin X server, so I can't
>> answer that question accurately.
>>
>> I shall put your previous patch into the next release and assuming it works
>> well, be pushing it upstream sometime before xserver 1.10
>>
>> I think this patch needs a little more work.
>>
>> I'm still a little concerned that there is no rate-limiting on the clipboard
>> restart mechanism, so in a pathological case (e.g. where some unanticipated
>> error causes the clipboard to constantly get disconnected), this will spin
>> using all available CPU.
>>
>> Some detailed comments below.
>>
>>> ---
>>> /home/hummelm/xserver-fc091936e2bddbbab9c9a501edc5a5f08388617e-org/hw/xwin/InitInput.c
>>>      2010-08-18 22:10:54.000000000 +0200
>>> +++ hw/xwin/InitInput.c 2010-09-17 17:03:08.611244200 +0200
>>> @@ -127,12 +127,6 @@ InitInput (int argc, char *argv[])
>>>       winProcEstablishConnectionOrig = InitialVector[2];
>>>       InitialVector[2] = winProcEstablishConnection;
>>>     }
>>> -  if (g_fXdmcpEnabled
>>> -      && ProcVector[X_QueryTree] != winProcQueryTree)
>>> -    {
>>> -      winProcQueryTreeOrig = ProcVector[X_QueryTree];
>>> -      ProcVector[X_QueryTree] = winProcQueryTree;
>>> -    }
>>>  #endif
>>>
>>>   g_pwinPointer = AddInputDevice (serverClient, winMouseProc, TRUE);
>>> ---
>>> /home/hummelm/xserver-fc091936e2bddbbab9c9a501edc5a5f08388617e-org/hw/xwin//winclipboardthread.c
>>>    2010-08-18 22:10:54.000000000 +0200
>>> +++ hw/xwin/winclipboardthread.c        2010-09-24 16:48:20.689125100
>>> +0200
>>> @@ -48,6 +48,8 @@
>>>  extern Bool            g_fUnicodeClipboard;
>>>  extern unsigned long   serverGeneration;
>>>  extern Bool            g_fClipboardStarted;
>>> +extern Bool             g_fClipboardLaunched;
>>> +extern Bool             g_fClipboard;
>>>  extern HWND            g_hwndClipboard;
>>>  extern void            *g_pClipboardDisplay;
>>>  extern Window          g_iClipboardWindow;
>>> @@ -72,8 +74,113 @@ winClipboardErrorHandler (Display *pDisp
>>>  static int
>>>  winClipboardIOErrorHandler (Display *pDisplay);
>>>
>>> +static void
>>> +restartClipboardThread(void *pvNotUsed);
>>> +
>>> +static void
>>> +winClipboardCleanup(void *vfdMessageQueue);
>>> +
>>>
>>>  /*
>>> + * restartClipboardThread activate the ProcEstablishConnection wrapper
>>> which will start
>>> + * the thread after next client connection
>>> + */
>>
>> This comment doesn't appear to be correct.  winInitClipboard() starts the
>> thread itself.
>>
>> It looks like this could fight with the ProcEstablishConnection wrapper,
>> which will call winInitClipboard() once for each server generation.
>>
>>> +
>>> +static void restartClipboardThread(void *pvNotUsed)
>>> +{
>>> +  winDebug("restartClipboardThread - enter \n");
>>> +  if (g_fClipboard)
>>> +    {
>>> +      ErrorF("restartClipboardThread - trying to restart clipboard thread
>>> \n");
>>> +      /* Create the clipboard client thread */
>>> +      if (!winInitClipboard ())
>>> +        {
>>> +          ErrorF ("restartClipboardThread - winClipboardInit "
>>> +                  "failed.\n");
>>> +          return;
>>> +        }
>>> +
>>> +      winDebug ("restartClipboardThread - winInitClipboard returned.\n");
>>> +      /* Flag that clipboard client has been launched */
>>> +      g_fClipboardLaunched = TRUE;
>>> +    }
>>> +  else
>>> +    {
>>> +      ErrorF ("restartClipboardThread - Clipboard disabled  - Exit from
>>> server \n");
>>> +      return;
>>> +    }
>>> +  return;
>>> +}
>>> +
>>> +/*
>>> + * winClipboardCleanup clean thread before exit
>>> + */
>>> +
>>> +static void winClipboardCleanup(void *vfdMessageQueue)
>>> +{
>>> +  int iReturn;
>>> +  int fdMessageQueue = (int) vfdMessageQueue;
>>> +
>>> +  winDebug("winClipboardCleanup - Enter \n");
>>> +  CloseClipboard();
>>> +  /* Close our Windows window */
>>> +  if (g_hwndClipboard )
>>> +    {
>>> +      /* Destroy the Window window (hwnd) */
>>> +      winDebug("winClipboardCleanup - Destroy Windows window\n");
>>> +      PostMessage (g_hwndClipboard,WM_DESTROY, 0, 0);
>>> +      winClipboardFlushWindowsMessageQueue(g_hwndClipboard);
>>> +    }
>>> +
>>> +  /* Close our X window */
>>> +  if (g_pClipboardDisplay && g_iClipboardWindow)
>>> +    {
>>> +      iReturn = XDestroyWindow (g_pClipboardDisplay, g_iClipboardWindow);
>>> +      if (iReturn == BadWindow)
>>> +       ErrorF ("winClipboardCleanup - XDestroyWindow returned
>>> BadWindow.\n");
>>> +      else
>>> +       winDebug ("winClipboardCleanup - winClipboardProc - XDestroyWindow
>>> succeeded.\n");
>>> +    }
>>> +
>>> +
>>> +#ifdef HAS_DEVWINDOWS
>>> +  /* Close our Win32 message handle */
>>> +  if (fdMessageQueue)
>>> +    close (fdMessageQueue);
>>> +#endif
>>> +
>>> +#if 0
>>> +  /*
>>> +   * FIXME: XCloseDisplay hangs if we call it, as of 2004/03/26.  The
>>> +   * XSync and XSelectInput calls did not help.
>>> +   */
>>> +
>>> +  /* Discard any remaining events */
>>> +  XSync (g_pClipboardDisplay, TRUE);
>>> +
>>> +  /* Select event types to watch */
>>> +  XSelectInput (g_pClipboardDisplay,
>>> +               DefaultRootWindow (g_pClipboardDisplay),
>>> +               None);
>>> +
>>> +  /* Close our X display */
>>> +  if (g_pClipboardDisplay)
>>> +    {
>>> +      XCloseDisplay (g_pClipboardDisplay);
>>> +    }
>>> +#endif
>>> +
>>> +  /* global clipboard variable reset */
>>> +  g_fClipboardLaunched = FALSE;
>>> +  g_fClipboardStarted = FALSE;
>>> +  g_iClipboardWindow = None;
>>> +  g_hwndClipboard = NULL;
>>> +  g_fUnicodeClipboard = FALSE;
>>> +  g_pClipboardDisplay = NULL;
>>> +  winDebug("winClipboardCleanup - Exit \n");
>>> +
>>> +}
>>
>> Rather than 2 separate functions which we pthread_cleanup_push separately,
>> but never use independently, can these be combined?  Or create a cleanup fn
>> which calls both of them, and push that?
>>
>>> +/*
>>>  * Main thread function
>>>  */
>>>
>>> @@ -84,9 +191,8 @@ winClipboardProc (void *pvNotUsed)
>>>   int                  iReturn;
>>>   HWND                 hwnd = NULL;
>>>   int                  iConnectionNumber = 0;
>>> -#ifdef HAS_DEVWINDOWS
>>>   int                  fdMessageQueue = 0;
>>> -#else
>>> +#ifndef HAS_DEVWINDOWS
>>>   struct timeval        tvTimeout;
>>>  #endif
>>>   fd_set               fdsRead;
>>> @@ -122,24 +228,6 @@ winClipboardProc (void *pvNotUsed)
>>>       ErrorF ("winClipboardProc - Warning: Locale not supported by X.\n");
>>>     }
>>>
>>> -  /* Set jump point for Error exits */
>>> -  iReturn = setjmp (g_jmpEntry);
>>> -
>>> -  /* Check if we should continue operations */
>>> -  if (iReturn != WIN_JMP_ERROR_IO
>>> -      && iReturn != WIN_JMP_OKAY)
>>> -    {
>>> -      /* setjmp returned an unknown value, exit */
>>> -      ErrorF ("winClipboardProc - setjmp returned: %d exiting\n",
>>> -             iReturn);
>>> -      pthread_exit (NULL);
>>> -    }
>>> -  else if (iReturn == WIN_JMP_ERROR_IO)
>>> -    {
>>> -      /* TODO: Cleanup the Win32 window and free any allocated memory */
>>> -      ErrorF ("winClipboardProc - setjmp returned for IO Error
>>> Handler.\n");
>>> -      pthread_exit (NULL);
>>> -    }
>>>
>>>   /* Use our generated cookie for authentication */
>>>   winSetAuthorization();
>>> @@ -216,6 +304,25 @@ winClipboardProc (void *pvNotUsed)
>>>   iMaxDescriptor = iConnectionNumber + 1;
>>>  #endif
>>>
>>> +  /* Adding a cleanup process for thread exit
>>> +   * Warning : A call to pthread_cleanup_push() implies a call to
>>> pthread_cleanup_pop() at the same code level (function)
>>> +   * We also use it to automaticaly restart the thread on unexpected exit
>>> (ie. pthread_exit() when g_pClipboard != TRUE )
>>> +   * this is a LIFO stack ( Last In First Out ) so adding "restart" then
>>> "clean"
>>> +   */
>>> +  /* Install the restart function  to be called */
>>> +  pthread_cleanup_push(restartClipboardThread,  NULL);
>>> +  /*  install the cleanup function to be called at thread exit */
>>> +  pthread_cleanup_push(winClipboardCleanup, (void*) &fdMessageQueue);
>>> +  /* The only way to exit from this loop is to :
>>> +   * 1/ Exit before the installation this cleanup process
>>> +   * 2/ Doing the "expected" Exit (ie. End of the function) which disable
>>> the restart
>>> +   *    by setting g_pClipboard to FALSE;
>>> +   *    before calling the cleanup handler :
>>> +   *  pthread_cleanup_pop(1);
>>> +   *    then the restart handler which will be an Exit handler because
>>> g_pClipboard != TRUE :
>>> +   *  pthread_cleanup_pop(1);
>>> +   */
>>> +
>>>   /* Create atoms */
>>>   atomClipboard = XInternAtom (pDisplay, "CLIPBOARD", False);
>>>   atomClipboardManager = XInternAtom (pDisplay, "CLIPBOARD_MANAGER",
>>> False);
>>> @@ -292,6 +399,26 @@ winClipboardProc (void *pvNotUsed)
>>>   /* Signal that the clipboard client has started */
>>>   g_fClipboardStarted = TRUE;
>>>
>>> +
>>> +  /* Set jump point for Error exits */
>>> +  iReturn = setjmp (g_jmpEntry);
>>> +
>>> +  /* Check if we should continue operations */
>>> +  if (iReturn != WIN_JMP_ERROR_IO
>>> +      && iReturn != WIN_JMP_OKAY)
>>> +    {
>>> +      /* setjmp returned an unknown value, exit */
>>> +      ErrorF ("winClipboardProc - setjmp returned: %d exiting\n",
>>> +             iReturn);
>>> +      pthread_exit (NULL);
>>> +    }
>>> +  else if (iReturn == WIN_JMP_ERROR_IO)
>>> +    {
>>> +      /* TODO: Cleanup the Win32 window and free any allocated memory */
>>> +      ErrorF ("winClipboardProc - setjmp returned for IO Error
>>> Handler.\n");
>>> +      pthread_exit (NULL);
>>> +    }
>>> +
>>>   /* Loop for X events */
>>>   while (1)
>>>     {
>>> @@ -377,47 +504,10 @@ winClipboardProc (void *pvNotUsed)
>>>        }
>>>     }
>>>
>>> -  /* Close our X window */
>>> -  if (pDisplay && iWindow)
>>> -    {
>>> -      iReturn = XDestroyWindow (pDisplay, iWindow);
>>> -      if (iReturn == BadWindow)
>>> -       ErrorF ("winClipboardProc - XDestroyWindow returned
>>> BadWindow.\n");
>>> -      else
>>> -       ErrorF ("winClipboardProc - XDestroyWindow succeeded.\n");
>>> -    }
>>> -
>>> -
>>> -#ifdef HAS_DEVWINDOWS
>>> -  /* Close our Win32 message handle */
>>> -  if (fdMessageQueue)
>>> -    close (fdMessageQueue);
>>> -#endif
>>> -
>>> -#if 0
>>> -  /*
>>> -   * FIXME: XCloseDisplay hangs if we call it, as of 2004/03/26.  The
>>> -   * XSync and XSelectInput calls did not help.
>>> -   */
>>> -
>>> -  /* Discard any remaining events */
>>> -  XSync (pDisplay, TRUE);
>>> -
>>> -  /* Select event types to watch */
>>> -  XSelectInput (pDisplay,
>>> -               DefaultRootWindow (pDisplay),
>>> -               None);
>>> -
>>> -  /* Close our X display */
>>> -  if (pDisplay)
>>> -    {
>>> -      XCloseDisplay (pDisplay);
>>> -    }
>>> -#endif
>>> -
>>> -  g_iClipboardWindow = None;
>>> -  g_pClipboardDisplay = NULL;
>>> -  g_hwndClipboard = NULL;
>>> +  /* disable the clipboard means thread restart function will kill the
>>> server */
>>> +  g_fClipboard = FALSE;
>>> +  pthread_cleanup_pop(1);
>>> +  pthread_cleanup_pop(1);
>>
>> The use of pthread_cleanup really obscures what's going on here.  It would
>> be clearer just to have a label error_exit: here and goto it from the
>> various places which pthread_exit...
>>
>>>
>>>   return NULL;
>>>  }
>>> @@ -431,7 +521,7 @@ static int
>>>  winClipboardErrorHandler (Display *pDisplay, XErrorEvent *pErr)
>>>  {
>>>   char pszErrorMsg[100];
>>> -
>>> +
>>
>> No unnecessary whitespace changes, please.
>>
>>>   XGetErrorText (pDisplay,
>>>                 pErr->error_code,
>>>                 pszErrorMsg,
>>> @@ -442,6 +532,13 @@ winClipboardErrorHandler (Display *pDisp
>>>          pErr->serial,
>>>          pErr->request_code,
>>>          pErr->minor_code);
>>> +
>>> +  /* If the Xerror is BadWindow Error, restart the thread */
>>> +  if ( pErr->request_code == 24 )
>>> +   {
>>> +     ErrorF("winClipboardErrorHandler - Badwindow detected, restarting
>>> clipboard thread\n");
>>> +     pthread_exit(NULL);
>>> +   }
>>
>> How do we get into this situation?  Window has been deleted by someone else,
>> but we are still connected to the server?
>>
>>>   return 0;
>>>  }
>>>
>>> ---
>>> /home/hummelm/xserver-fc091936e2bddbbab9c9a501edc5a5f08388617e-org/hw/xwin/winclipboardwrappers.c
>>>   2010-08-18 22:10:54.000000000 +0200
>>> +++ hw/xwin/winclipboardwrappers.c      2010-09-22 10:06:00.232451900
>>> +0200
>>> @@ -53,7 +53,6 @@
>>>  */
>>>
>>>  DISPATCH_PROC(winProcEstablishConnection);
>>> -DISPATCH_PROC(winProcQueryTree);
>>>  DISPATCH_PROC(winProcSetSelectionOwner);
>>>
>>>
>>> @@ -79,104 +78,6 @@ extern winDispatchProcPtr   winProcSetSele
>>>
>>>
>>>  /*
>>> - * Wrapper for internal QueryTree function.
>>> - * Hides the clipboard client when it is the only client remaining.
>>> - */
>>> -
>>> -int
>>> -winProcQueryTree (ClientPtr client)
>>> -{
>>> -  int                  iReturn;
>>> -
>>> -  ErrorF ("winProcQueryTree - Hello\n");
>>> -
>>> -  /*
>>> -   * This procedure is only used for initialization.
>>> -   * We can unwrap the original procedure at this point
>>> -   * so that this function is no longer called until the
>>> -   * server resets and the function is wrapped again.
>>> -   */
>>> -  ProcVector[X_QueryTree] = winProcQueryTreeOrig;
>>> -
>>> -  /*
>>> -   * Call original function and bail if it fails.
>>> -   * NOTE: We must do this first, since we need XdmcpOpenDisplay
>>> -   * to be called before we initialize our clipboard client.
>>> -   */
>>> -  iReturn = (*winProcQueryTreeOrig) (client);
>>> -  if (iReturn != 0)
>>> -    {
>>> -      ErrorF ("winProcQueryTree - ProcQueryTree failed, bailing.\n");
>>> -      return iReturn;
>>> -    }
>>> -
>>> -  /* Make errors more obvious */
>>> -  winProcQueryTreeOrig = NULL;
>>> -
>>> -  /* Do nothing if clipboard is not enabled */
>>> -  if (!g_fClipboard)
>>> -    {
>>> -      ErrorF ("winProcQueryTree - Clipboard is not enabled, "
>>> -             "returning.\n");
>>> -      return iReturn;
>>> -    }
>>> -
>>> -  /* If the clipboard client has already been started, abort */
>>> -  if (g_fClipboardLaunched)
>>> -    {
>>> -      ErrorF ("winProcQueryTree - Clipboard client already "
>>> -             "launched, returning.\n");
>>> -      return iReturn;
>>> -    }
>>> -
>>> -  /* Startup the clipboard client if clipboard mode is being used */
>>> -  if (g_fXdmcpEnabled && g_fClipboard)
>>> -    {
>>> -      /*
>>> -       * NOTE: The clipboard client is started here for a reason:
>>> -       * 1) Assume you are using XDMCP (e.g. XWin -query %hostname%)
>>> -       * 2) If the clipboard client attaches during X Server startup,
>>> -       *    then it becomes the "magic client" that causes the X Server
>>> -       *    to reset if it exits.
>>> -       * 3) XDMCP calls KillAllClients when it starts up.
>>> -       * 4) The clipboard client is a client, so it is killed.
>>> -       * 5) The clipboard client is the "magic client", so the X Server
>>> -       *    resets itself.
>>> -       * 6) This repeats ad infinitum.
>>> -       * 7) We avoid this by waiting until at least one client (could
>>> -       *    be XDM, could be another client) connects, which makes it
>>> -       *    almost certain that the clipboard client will not connect
>>> -       *    until after XDM when using XDMCP.
>>> -       * 8) Unfortunately, there is another problem.
>>> -       * 9) XDM walks the list of windows with XQueryTree,
>>> -       *    killing any client it finds with a window.
>>> -       * 10)Thus, when using XDMCP we wait until the first call
>>> -       *    to ProcQueryTree before we startup the clipboard client.
>>> -       *    This should prevent XDM from finding the clipboard client,
>>> -       *    since it has not yet created a window.
>>> -       * 11)Startup when not using XDMCP is handled in
>>> -       *    winProcEstablishConnection.
>>> -       */
>>> -
>>> -      /* Create the clipboard client thread */
>>> -      if (!winInitClipboard ())
>>> -       {
>>> -         ErrorF ("winProcQueryTree - winClipboardInit "
>>> -                 "failed.\n");
>>> -         return iReturn;
>>> -       }
>>> -
>>> -      ErrorF ("winProcQueryTree - winInitClipboard returned.\n");
>>> -    }
>>> -
>>> -  /* Flag that clipboard client has been launched */
>>> -  g_fClipboardLaunched = TRUE;
>>> -
>>> -  return iReturn;
>>> -}
>>> -
>>> -
>>> -/*
>>>  * Wrapper for internal EstablishConnection function.
>>>  * Initializes internal clients that must not be started until
>>>  * an external client has connected.
>>> @@ -217,18 +118,6 @@ winProcEstablishConnection (ClientPtr cl
>>>   /* Increment call count */
>>>   ++s_iCallCount;
>>>
>>> -  /* Wait for CLIP_NUM_CALLS when Xdmcp is enabled */
>>> -  if (g_fXdmcpEnabled
>>> -      && !g_fClipboardLaunched
>>> -      && s_iCallCount < CLIP_NUM_CALLS)
>>> -    {
>>> -      if (s_iCallCount == 1) ErrorF ("winProcEstablishConnection - Xdmcp,
>>> waiting to "
>>> -             "start clipboard client until %dth call", CLIP_NUM_CALLS);
>>> -      if (s_iCallCount == CLIP_NUM_CALLS - 1) ErrorF (".\n");
>>> -      else ErrorF (".");
>>> -      return (*winProcEstablishConnectionOrig) (client);
>>> -    }
>>> -
>>>   /*
>>>    * This procedure is only used for initialization.
>>>    * We can unwrap the original procedure at this point
>>> @@ -279,13 +168,6 @@ winProcEstablishConnection (ClientPtr cl
>>>        *    be XDM, could be another client) connects, which makes it
>>>        *    almost certain that the clipboard client will not connect
>>>        *    until after XDM when using XDMCP.
>>> -       * 8) Unfortunately, there is another problem.
>>> -       * 9) XDM walks the list of windows with XQueryTree,
>>> -       *    killing any client it finds with a window.
>>> -       * 10)Thus, when using XDMCP we wait until CLIP_NUM_CALLS
>>> -       *    to ProcEstablishCeonnection before we startup the clipboard
>>> -       *    client.  This should prevent XDM from finding the clipboard
>>> -       *    client, since it has not yet created a window.
>>>        */
>>>
>>>       /* Create the clipboard client thread */
>>> @@ -442,7 +324,8 @@ winProcSetSelectionOwner (ClientPtr clie
>>>
>>>       /* Release ownership of the Windows clipboard */
>>>       OpenClipboard (NULL);
>>> -      EmptyClipboard ();
>>> +      /* on clipboard restart  EmptyClipboard (); makes the X server to
>>> freeze  for 1 or 2 seconds and I don't know why */
>>> +      /* EmptyClipboard (); */
>>>       CloseClipboard ();
>>>
>>>       goto winProcSetSelectionOwner_Done;
>>
>> --
>> Jon TURNEY
>> Volunteer Cygwin/X X Server maintainer
>>
>

[-- Attachment #2: XWin_patch_restart_clipboard1.9.patch --]
[-- Type: application/octet-stream, Size: 9246 bytes --]

--- /home/hummelm/xserver-980c36531228e953058c5b34b6bfaacf8cb1601a-org/hw/xwin/winclipboard.h	2010-09-28 17:22:50.400391900 +0200
+++ hw/xwin//winclipboard.h	2010-09-29 13:43:52.995248000 +0200
@@ -71,6 +71,8 @@
 #define WIN_XEVENTS_SHUTDOWN			1
 #define WIN_XEVENTS_CONVERT			2
 #define WIN_XEVENTS_NOTIFY			3
+#define WIN_CLIPBOARD_RETRIES			40
+#define WIN_CLIPBOARD_DELAY			1
 
 #define WM_WM_REINIT                           (WM_USER + 1)
 
--- /home/hummelm/xserver-980c36531228e953058c5b34b6bfaacf8cb1601a-org/hw/xwin/winclipboardthread.c	2010-09-28 17:22:50.416028100 +0200
+++ hw/xwin//winclipboardthread.c	2010-09-29 15:40:45.632202400 +0200
@@ -48,6 +48,8 @@
 extern Bool		g_fUnicodeClipboard;
 extern unsigned long	serverGeneration;
 extern Bool		g_fClipboardStarted;
+extern Bool             g_fClipboardLaunched;
+extern Bool             g_fClipboard;
 extern HWND		g_hwndClipboard;
 extern void		*g_pClipboardDisplay;
 extern Window		g_iClipboardWindow;
@@ -60,6 +62,7 @@ extern Window		g_iClipboardWindow;
 static jmp_buf			g_jmpEntry;
 static XIOErrorHandler g_winClipboardOldIOErrorHandler;
 static pthread_t g_winClipboardProcThread;
+static short clipboard_generation = 0;
 
 Bool				g_fUnicodeSupport = FALSE;
 Bool				g_fUseUnicode = FALSE;
@@ -89,9 +92,8 @@ winClipboardProc (void *pvNotUsed)
   int			iReturn;
   HWND			hwnd = NULL;
   int			iConnectionNumber = 0;
-#ifdef HAS_DEVWINDOWS
   int			fdMessageQueue = 0;
-#else
+#ifndef HAS_DEVWINDOWS
   struct timeval        tvTimeout;
 #endif
   fd_set		fdsRead;
@@ -103,9 +105,10 @@ winClipboardProc (void *pvNotUsed)
   char			szDisplay[512];
   int			iSelectError;
 
-  pthread_cleanup_push(&winClipboardThreadExit, NULL);
+  pthread_cleanup_push(&winClipboardThreadExit,  (void*) &fdMessageQueue);
 
   ErrorF ("winClipboardProc - Hello\n");
+  ++clipboard_generation;
 
   /* Do we have Unicode support? */
   g_fUnicodeSupport = winClipboardDetectUnicodeSupport ();
@@ -120,7 +123,7 @@ winClipboardProc (void *pvNotUsed)
   if (XInitThreads () == 0)
     {
       ErrorF ("winClipboardProc - XInitThreads failed.\n");
-      pthread_exit (NULL);
+      goto winClipboardProc_Done;
     }
 
   /* See if X supports the current locale */
@@ -144,7 +147,7 @@ winClipboardProc (void *pvNotUsed)
       /* setjmp returned an unknown value, exit */
       ErrorF ("winClipboardProc - setjmp returned: %d exiting\n",
 	      iReturn);
-      pthread_exit (NULL);
+      goto winClipboardProc_Done;
     }
   else if (iReturn == WIN_JMP_ERROR_IO)
     {
@@ -194,7 +197,7 @@ winClipboardProc (void *pvNotUsed)
   if (pDisplay == NULL)
     {
       ErrorF ("winClipboardProc - Failed opening the display, giving up\n");
-      pthread_exit (NULL);
+      goto winClipboardProc_Done;
     }
 
   /* Save the display in a global used by the wndproc */
@@ -212,7 +215,7 @@ winClipboardProc (void *pvNotUsed)
   if (fdMessageQueue == -1)
     {
       ErrorF ("winClipboardProc - Failed opening %s\n", WIN_MSG_QUEUE_FNAME);
-      pthread_exit (NULL);
+      goto winClipboardProc_Done;
     }
 
   /* Find max of our file descriptors */
@@ -236,7 +239,7 @@ winClipboardProc (void *pvNotUsed)
   if (iWindow == 0)
     {
       ErrorF ("winClipboardProc - Could not create an X window.\n");
-      pthread_exit (NULL);
+      goto winClipboardProc_Done;
     }
 
   /* Select event types to watch */
@@ -265,7 +268,7 @@ winClipboardProc (void *pvNotUsed)
 	  XGetSelectionOwner (pDisplay, XA_PRIMARY) != iWindow)
 	{
 	  ErrorF ("winClipboardProc - Could not set PRIMARY owner\n");
-	  pthread_exit (NULL);
+          goto winClipboardProc_Done;
 	}
 
       /* CLIPBOARD */
@@ -275,7 +278,7 @@ winClipboardProc (void *pvNotUsed)
 	  XGetSelectionOwner (pDisplay, atomClipboard) != iWindow)
 	{
 	  ErrorF ("winClipboardProc - Could not set CLIPBOARD owner\n");
-	  pthread_exit (NULL);
+          goto winClipboardProc_Done;
 	}
     }
 
@@ -294,7 +297,7 @@ winClipboardProc (void *pvNotUsed)
   if (!winClipboardFlushWindowsMessageQueue (hwnd))
     {
       ErrorF ("winClipboardProc - winClipboardFlushWindowsMessageQueue failed\n");
-      pthread_exit (NULL);
+      goto winClipboardProc_Done;
     }
 
   /* Signal that the clipboard client has started */
@@ -385,50 +388,11 @@ winClipboardProc (void *pvNotUsed)
 	}
     }
 
-winClipboardProc_Done:
-  /* Close our X window */
-  if (pDisplay && iWindow)
-    {
-      iReturn = XDestroyWindow (pDisplay, iWindow);
-      if (iReturn == BadWindow)
-	ErrorF ("winClipboardProc - XDestroyWindow returned BadWindow.\n");
-      else
-	ErrorF ("winClipboardProc - XDestroyWindow succeeded.\n");
-    }
-
-
-#ifdef HAS_DEVWINDOWS
-  /* Close our Win32 message handle */
-  if (fdMessageQueue)
-    close (fdMessageQueue);
-#endif
+  /* disable the clipboard means thread will dies */
+  g_fClipboard = FALSE;
 
-#if 0
-  /*
-   * FIXME: XCloseDisplay hangs if we call it, as of 2004/03/26.  The
-   * XSync and XSelectInput calls did not help.
-   */
-
-  /* Discard any remaining events */
-  XSync (pDisplay, TRUE);
-
-  /* Select event types to watch */
-  XSelectInput (pDisplay,
-		DefaultRootWindow (pDisplay),
-		None);
-
-  /* Close our X display */
-  if (pDisplay)
-    {
-      XCloseDisplay (pDisplay);
-    }
-#endif
-
-  g_iClipboardWindow = None;
-  g_pClipboardDisplay = NULL;
-  g_hwndClipboard = NULL;
-
-  pthread_cleanup_pop(0);
+winClipboardProc_Done:
+  pthread_cleanup_pop(1);
 
   return NULL;
 }
@@ -453,6 +417,16 @@ winClipboardErrorHandler (Display *pDisp
 	  pErr->serial,
 	  pErr->request_code,
 	  pErr->minor_code);
+  if (pthread_equal(pthread_self(),g_winClipboardProcThread) &&  ( pErr->request_code == 24 ) )
+    {
+      /* We are in the clipboard thread and a bad window is detected.
+       * This means the Xwindow part of the clipboard doesn't exist anymore 
+       * The call to "pthread_exit(NULL)" will restart the thread 
+       */
+       ErrorF("winClipboardErrorHandler - Badwindow detected, restarting clipboard thread\n");
+       pthread_exit(NULL);
+    }
+
   return 0;
 }
 
@@ -480,11 +454,104 @@ winClipboardIOErrorHandler (Display *pDi
 
 /*
  * winClipboardThreadExit - Thread exit handler
+ * Cleanup the thread and restart if needed
  */
 
 static void
-winClipboardThreadExit(void *arg)
+winClipboardThreadExit(void *vfdMessageQueue)
 {
-  /* clipboard thread has exited, stop server as well */
-  kill(getpid(), SIGTERM);
+  int iReturn;
+  int fdMessageQueue = (int) vfdMessageQueue;
+
+  winDebug("winClipboardThreadExit - enter \n");
+
+ /* First clean the thread */
+  CloseClipboard();
+  /* Close our Windows window */
+  if (g_hwndClipboard )
+    {
+      /* Destroy the Window window (hwnd) */
+      winDebug("winClipboardThreadExit - Destroy Windows window\n");
+      PostMessage (g_hwndClipboard,WM_DESTROY, 0, 0);
+      winClipboardFlushWindowsMessageQueue(g_hwndClipboard);
+    }
+
+  /* Close our X window */
+  if (g_pClipboardDisplay && g_iClipboardWindow)
+    {
+      iReturn = XDestroyWindow (g_pClipboardDisplay, g_iClipboardWindow);
+      if (iReturn == BadWindow)
+        ErrorF ("winClipboardThreadExit - XDestroyWindow returned BadWindow.\n");
+      else
+        winDebug ("winClipboardThreadExit - winClipboardProc - XDestroyWindow succeeded.\n");
+    }
+
+
+#ifdef HAS_DEVWINDOWS
+  /* Close our Win32 message handle */
+  if (fdMessageQueue)
+    close (fdMessageQueue);
+#endif
+
+#if 0
+  /*
+   * FIXME: XCloseDisplay hangs if we call it, as of 2004/03/26.  The
+   * XSync and XSelectInput calls did not help.
+   */
+
+  /* Discard any remaining events */
+  XSync (g_pClipboardDisplay, TRUE);
+
+  /* Select event types to watch */
+  XSelectInput (g_pClipboardDisplay,
+                DefaultRootWindow (g_pClipboardDisplay),
+                None);
+
+  /* Close our X display */
+  if (g_pClipboardDisplay)
+    {
+      XCloseDisplay (g_pClipboardDisplay);
+    }
+#endif
+ 
+  /* global clipboard variable reset */
+  g_fClipboardLaunched = FALSE;
+  g_fClipboardStarted = FALSE;
+  g_iClipboardWindow = None;
+  g_hwndClipboard = NULL;
+  g_fUnicodeClipboard = FALSE;
+  g_pClipboardDisplay = NULL;
+
+  /* checking if we need to restart */
+  if (clipboard_generation >= WIN_CLIPBOARD_RETRIES) 
+    {
+      /* terminates clipboard thread but the main server still lives */
+      ErrorF("winClipboardThreadExit - the clipboard thread seems to be unstable, the clipboard thread will now exit\n");
+      g_fClipboard = FALSE;
+      return;
+    }
+
+  if (g_fClipboard)
+    {
+      sleep(WIN_CLIPBOARD_DELAY);
+      ErrorF("winClipboardThreadExit - trying to restart clipboard thread \n");
+      /* Create the clipboard client thread */
+      if (!winInitClipboard ())
+        {
+          ErrorF ("winClipboardThreadExit - winClipboardInit "
+                  "failed.\n");
+          return;
+        }
+ 
+      winDebug ("winClipboardThreadExit - winInitClipboard returned.\n");
+      /* Flag that clipboard client has been launched */
+      g_fClipboardLaunched = TRUE;
+    }
+  else
+    {
+      ErrorF ("winClipboardThreadExit - Clipboard disabled  - Exit from server \n");
+      /* clipboard thread has exited, stop server as well */
+      kill(getpid(), SIGTERM);
+    }
+  return;  
 }

[-- Attachment #3: XWin_patch_remove_xdmcp_tricks1.9.patch --]
[-- Type: application/octet-stream, Size: 6250 bytes --]

--- /home/hummelm/xserver-980c36531228e953058c5b34b6bfaacf8cb1601a-org/hw/xwin/InitInput.c	2010-09-28 17:22:50.353483300 +0200
+++ hw/xwin//InitInput.c	2010-09-28 18:15:52.327041200 +0200
@@ -60,10 +60,8 @@ DeviceIntPtr g_pwinKeyboard;
 #ifdef HAS_DEVWINDOWS
 extern int			g_fdMessageQueue;
 #endif
-extern Bool			g_fXdmcpEnabled;
 #ifdef XWIN_CLIPBOARD
 extern winDispatchProcPtr	winProcEstablishConnectionOrig;
-extern winDispatchProcPtr	winProcQueryTreeOrig;
 #endif
 
 
@@ -127,12 +125,6 @@ InitInput (int argc, char *argv[])
       winProcEstablishConnectionOrig = InitialVector[2];
       InitialVector[2] = winProcEstablishConnection;
     }
-  if (g_fXdmcpEnabled
-      && ProcVector[X_QueryTree] != winProcQueryTree)
-    {
-      winProcQueryTreeOrig = ProcVector[X_QueryTree];
-      ProcVector[X_QueryTree] = winProcQueryTree;
-    }
 #endif
 
   g_pwinPointer = AddInputDevice (serverClient, winMouseProc, TRUE);
--- /home/hummelm/xserver-980c36531228e953058c5b34b6bfaacf8cb1601a-org/hw/xwin/winclipboardwrappers.c	2010-09-28 17:22:50.447300500 +0200
+++ hw/xwin//winclipboardwrappers.c	2010-09-29 15:10:50.466764400 +0200
@@ -53,7 +53,6 @@
  */
 
 DISPATCH_PROC(winProcEstablishConnection);
-DISPATCH_PROC(winProcQueryTree);
 DISPATCH_PROC(winProcSetSelectionOwner);
 
 
@@ -79,104 +78,6 @@ extern winDispatchProcPtr	winProcSetSele
 
 
 /*
- * Wrapper for internal QueryTree function.
- * Hides the clipboard client when it is the only client remaining.
- */
-
-int
-winProcQueryTree (ClientPtr client)
-{
-  int			iReturn;
-
-  ErrorF ("winProcQueryTree - Hello\n");
-
-  /*
-   * This procedure is only used for initialization.
-   * We can unwrap the original procedure at this point
-   * so that this function is no longer called until the
-   * server resets and the function is wrapped again.
-   */
-  ProcVector[X_QueryTree] = winProcQueryTreeOrig;
-
-  /*
-   * Call original function and bail if it fails.
-   * NOTE: We must do this first, since we need XdmcpOpenDisplay
-   * to be called before we initialize our clipboard client.
-   */
-  iReturn = (*winProcQueryTreeOrig) (client);
-  if (iReturn != 0)
-    {
-      ErrorF ("winProcQueryTree - ProcQueryTree failed, bailing.\n");
-      return iReturn;
-    }
-
-  /* Make errors more obvious */
-  winProcQueryTreeOrig = NULL;
-
-  /* Do nothing if clipboard is not enabled */
-  if (!g_fClipboard)
-    {
-      ErrorF ("winProcQueryTree - Clipboard is not enabled, "
-	      "returning.\n");
-      return iReturn;
-    }
-
-  /* If the clipboard client has already been started, abort */
-  if (g_fClipboardLaunched)
-    {
-      ErrorF ("winProcQueryTree - Clipboard client already "
-	      "launched, returning.\n");
-      return iReturn;
-    }
-
-  /* Startup the clipboard client if clipboard mode is being used */
-  if (g_fXdmcpEnabled && g_fClipboard)
-    {
-      /*
-       * NOTE: The clipboard client is started here for a reason:
-       * 1) Assume you are using XDMCP (e.g. XWin -query %hostname%)
-       * 2) If the clipboard client attaches during X Server startup,
-       *    then it becomes the "magic client" that causes the X Server
-       *    to reset if it exits.
-       * 3) XDMCP calls KillAllClients when it starts up.
-       * 4) The clipboard client is a client, so it is killed.
-       * 5) The clipboard client is the "magic client", so the X Server
-       *    resets itself.
-       * 6) This repeats ad infinitum.
-       * 7) We avoid this by waiting until at least one client (could
-       *    be XDM, could be another client) connects, which makes it
-       *    almost certain that the clipboard client will not connect
-       *    until after XDM when using XDMCP.
-       * 8) Unfortunately, there is another problem.
-       * 9) XDM walks the list of windows with XQueryTree,
-       *    killing any client it finds with a window.
-       * 10)Thus, when using XDMCP we wait until the first call
-       *    to ProcQueryTree before we startup the clipboard client.
-       *    This should prevent XDM from finding the clipboard client,
-       *    since it has not yet created a window.
-       * 11)Startup when not using XDMCP is handled in
-       *    winProcEstablishConnection.
-       */
-      
-      /* Create the clipboard client thread */
-      if (!winInitClipboard ())
-	{
-	  ErrorF ("winProcQueryTree - winClipboardInit "
-		  "failed.\n");
-	  return iReturn;
-	}
-      
-      ErrorF ("winProcQueryTree - winInitClipboard returned.\n");
-    }
-  
-  /* Flag that clipboard client has been launched */
-  g_fClipboardLaunched = TRUE;
-
-  return iReturn;
-}
-
-
-/*
  * Wrapper for internal EstablishConnection function.
  * Initializes internal clients that must not be started until
  * an external client has connected.
@@ -217,18 +118,6 @@ winProcEstablishConnection (ClientPtr cl
   /* Increment call count */
   ++s_iCallCount;
 
-  /* Wait for CLIP_NUM_CALLS when Xdmcp is enabled */
-  if (g_fXdmcpEnabled
-      && !g_fClipboardLaunched
-      && s_iCallCount < CLIP_NUM_CALLS)
-    {
-      if (s_iCallCount == 1) ErrorF ("winProcEstablishConnection - Xdmcp, waiting to "
-	      "start clipboard client until %dth call", CLIP_NUM_CALLS);
-      if (s_iCallCount == CLIP_NUM_CALLS - 1) ErrorF (".\n");
-      else ErrorF (".");
-      return (*winProcEstablishConnectionOrig) (client);
-    }
-
   /*
    * This procedure is only used for initialization.
    * We can unwrap the original procedure at this point
@@ -279,13 +168,6 @@ winProcEstablishConnection (ClientPtr cl
        *    be XDM, could be another client) connects, which makes it
        *    almost certain that the clipboard client will not connect
        *    until after XDM when using XDMCP.
-       * 8) Unfortunately, there is another problem.
-       * 9) XDM walks the list of windows with XQueryTree,
-       *    killing any client it finds with a window.
-       * 10)Thus, when using XDMCP we wait until CLIP_NUM_CALLS
-       *    to ProcEstablishCeonnection before we startup the clipboard
-       *    client.  This should prevent XDM from finding the clipboard
-       *    client, since it has not yet created a window.
        */
       
       /* Create the clipboard client thread */

[-- Attachment #4: Type: text/plain, Size: 223 bytes --]

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