XWin/Clipboard/Crash/Hang - Patch Take 2
Harold L Hunt II
huntharo@msu.edu
Wed May 12 07:10:00 GMT 2004
Dan,
Thanks much for the patch. I'm glad Alexander committed it already.
I'll try to get a release out soon since I am moving on Friday and won't
have my computer for at least a week.
Thanks for contributing,
Harold
Wilks, Dan wrote:
> Harold L Hunt II wrote:
>
>
>>I'm including the small fix from winclipboardwndproc.c in the next
>>release, but the changes to the debug messages should probably be
>>done using the newer logging functions that accept a log level and
>>verb.
>
>
> I think the small fix got lost. At least I couldn't find it in
> the clipboard window proc. No worries it gave me time to look into
> the problem some more.
>
> This patch should solve XWin crashing (silently quitting) while
> copying or pasting after using another Windows application which
> utilizes the clipboard chain. Remote desktop is one such application.
> If further crashes occur using XWin -logverbose 3 will be able to generate
> hopefully useful information from the clipboard.
>
> ChangeLog:
>
> * winclipboard.h: Add extern prototypes for winDebug, winErrorFVerb
> copied from winmsg.h.
> * winclipboardinit.c (winFixClipboardChain): Post rather than send the
> reinit message to the clipboard window. Sending the message caused,
> or possibly just exacerbated an existing, race condition that would
> cause the X server to hang when coming back from a remote desktop
> session.
> * winclipboardwndproc.c (winProcessXEventsTimeout): switch to new
> logging api's.
> (winClipboardWindowProc): switch to new logging api's. Add some
> additional debug logging. Make best effort to prevent our window
> appearing twice in the clipboard chain. Also detect loops when they
> occur and try to behave in a reasonable way.
>
>
> Patch:
>
> Index: winclipboard.h
> ===================================================================
> RCS file: /cvs/xorg/xc/programs/Xserver/hw/xwin/winclipboard.h,v
> retrieving revision 1.1.4.1.2.14
> diff -u -p -r1.1.4.1.2.14 winclipboard.h
> --- winclipboard.h 23 Apr 2004 18:17:29 -0000 1.1.4.1.2.14
> +++ winclipboard.h 7 May 2004 15:32:27 -0000
> @@ -84,6 +84,8 @@
>
> extern char *display;
> extern void ErrorF (const char* /*f*/, ...);
> +extern void winDebug (const char *format, ...);
> +extern void winErrorFVerb (int verb, const char *format, ...);
>
>
> /*
> Index: winclipboardinit.c
> ===================================================================
> RCS file: /cvs/xorg/xc/programs/Xserver/hw/xwin/winclipboardinit.c,v
> retrieving revision 1.1.4.1.2.8
> diff -u -p -r1.1.4.1.2.8 winclipboardinit.c
> --- winclipboardinit.c 23 Apr 2004 18:17:29 -0000 1.1.4.1.2.8
> +++ winclipboardinit.c 7 May 2004 15:32:27 -0000
> @@ -135,6 +135,6 @@ winFixClipboardChain (void)
> if (g_fClipboard
> && g_hwndClipboard)
> {
> - SendMessage (g_hwndClipboard, WM_WM_REINIT, 0, 0);
> + PostMessage (g_hwndClipboard, WM_WM_REINIT, 0, 0);
> }
> }
> Index: winclipboardwndproc.c
> ===================================================================
> RCS file: /cvs/xorg/xc/programs/Xserver/hw/xwin/winclipboardwndproc.c,v
> retrieving revision 1.1.4.1.2.20
> diff -u -p -r1.1.4.1.2.20 winclipboardwndproc.c
> --- winclipboardwndproc.c 26 Apr 2004 19:53:49 -0000 1.1.4.1.2.20
> +++ winclipboardwndproc.c 7 May 2004 15:32:27 -0000
> @@ -102,8 +102,8 @@ winProcessXEventsTimeout (HWND hwnd, int
> &tv); /* No timeout */
> if (iReturn <= 0)
> {
> - ErrorF ("winProcessXEventsTimeout - Call to select () failed: %d.
> "
> - "Bailing.\n", iReturn);
> + winErrorFVerb (1, "winProcessXEventsTimeout - Call to select () "
> + "failed: %d. Bailing.\n", iReturn);
> break;
> }
>
> @@ -145,9 +145,7 @@ winClipboardWindowProc (HWND hwnd, UINT
> {
> case WM_DESTROY:
> {
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_DESTROY\n");
> -#endif
> + winDebug ("winClipboardWindowProc - WM_DESTROY\n");
>
> /* Remove ourselves from the clipboard chain */
> ChangeClipboardChain (hwnd, s_hwndNextViewer);
> @@ -161,24 +159,61 @@ winClipboardWindowProc (HWND hwnd, UINT
>
> case WM_CREATE:
> {
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_CREATE\n");
> -#endif
> + winDebug ("winClipboardWindowProc - WM_CREATE\n");
>
> /* Add ourselves to the clipboard viewer chain */
> s_hwndNextViewer = SetClipboardViewer (hwnd);
> + if (s_hwndNextViewer == hwnd)
> + {
> + s_hwndNextViewer = NULL;
> + winErrorFVerb (1, "winClipboardWindowProc - WM_CREATE: "
> + "attempted to set next window to ourselves.");
> + }
> }
> return 0;
>
>
> case WM_CHANGECBCHAIN:
> {
> + static Bool s_fProcessingChangeCBChain = FALSE;
> + winDebug ("winClipboardWindowProc - WM_CHANGECBCHAIN: wParam(%x) "
> + "lParam(%x) s_hwndNextViewer(%x)\n",
> + wParam, lParam, s_hwndNextViewer);
> +
> +
> + /*
> + * We've occasionally seen a loop in the clipboard chain. Break
> + * it on the first hint of recursion.
> + */
> + if (! s_fProcessingChangeCBChain)
> + {
> + s_fProcessingChangeCBChain = TRUE;
> + }
> + else
> + {
> + winErrorFVerb (1, "winClipboardWindowProc - WM_CHANGECBCHAIN - "
> + "Nested calls detected. Bailing.\n");
> + winDebug ("winClipboardWindowProc - WM_CHANGECBCHAIN: Exit\n");
> + return 0;
> + }
> +
> if ((HWND) wParam == s_hwndNextViewer)
> - s_hwndNextViewer = (HWND) lParam;
> + {
> + s_hwndNextViewer = (HWND) lParam;
> + if (s_hwndNextViewer == hwnd)
> + {
> + s_hwndNextViewer = NULL;
> + winErrorFVerb (1, "winClipboardWindowProc -
> WM_CHANGECBCHAIN: "
> + "attempted to set next window to
> ourselves.");
> + }
> + }
> else if (s_hwndNextViewer)
> SendMessage (s_hwndNextViewer, message,
> wParam, lParam);
> +
> + s_fProcessingChangeCBChain = FALSE;
> }
> + winDebug ("winClipboardWindowProc - WM_CHANGECBCHAIN: Exit\n");
> return 0;
>
> case WM_WM_REINIT:
> @@ -196,21 +231,58 @@ winClipboardWindowProc (HWND hwnd, UINT
> * expensive than just putting ourselves back into the chain.
> */
>
> - s_fCBCInitialized = FALSE;
> - ChangeClipboardChain (hwnd, s_hwndNextViewer);
> - s_hwndNextViewer = NULL;
> - s_fCBCInitialized = FALSE;
> - s_hwndNextViewer = SetClipboardViewer (hwnd);
> + winDebug ("winClipboardWindowProc - WM_WM_REINIT: Enter\n");
> + if (hwnd != GetClipboardViewer ())
> + {
> + winDebug (" WM_WM_REINIT: Replacing us(%x) with %x at head "
> + "of chain\n", hwnd, s_hwndNextViewer);
> + s_fCBCInitialized = FALSE;
> + ChangeClipboardChain (hwnd, s_hwndNextViewer);
> + s_hwndNextViewer = NULL;
> + s_fCBCInitialized = FALSE;
> + winDebug (" WM_WM_REINIT: Putting us back at head of
> chain.\n");
> + s_hwndNextViewer = SetClipboardViewer (hwnd);
> + if (s_hwndNextViewer == hwnd)
> + {
> + s_hwndNextViewer = NULL;
> + winErrorFVerb (1, "winClipboardWindowProc - WM_WM_REINIT: "
> + "attempted to set next window to
> ourselves.\n");
> + }
> + }
> + else
> + {
> + winDebug (" WM_WM_REINIT: already at head of viewer chain.\n");
> + }
> }
> + winDebug ("winClipboardWindowProc - WM_WM_REINIT: Exit\n");
> return 0;
>
>
> case WM_DRAWCLIPBOARD:
> {
> + static Bool s_fProcessingDrawClipboard = FALSE;
> Display *pDisplay = g_pClipboardDisplay;
> Window iWindow = g_iClipboardWindow;
> int iReturn;
>
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD: Enter\n");
> +
> + /*
> + * We've occasionally seen a loop in the clipboard chain. Break
> + * it on the first hint of recursion.
> + */
> + if (! s_fProcessingDrawClipboard)
> + {
> + s_fProcessingDrawClipboard = TRUE;
> + }
> + else
> + {
> + winErrorFVerb (1, "winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + "Nested calls detected. Bailing.\n");
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD: Exit\n");
> + return 0;
> + }
> +
> /* Pass the message on the next window in the clipboard viewer chain
> */
> if (s_hwndNextViewer)
> SendMessage (s_hwndNextViewer, message, 0, 0);
> @@ -219,6 +291,8 @@ winClipboardWindowProc (HWND hwnd, UINT
> if (!s_fCBCInitialized)
> {
> s_fCBCInitialized = TRUE;
> + s_fProcessingDrawClipboard = FALSE;
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD: Exit\n");
> return 0;
> }
>
> @@ -233,10 +307,11 @@ winClipboardWindowProc (HWND hwnd, UINT
> /* Bail when we still own the clipboard */
> if (hwnd == GetClipboardOwner ())
> {
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> +
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "We own the clipboard, returning.\n");
> -#endif
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD: Exit\n");
> + s_fProcessingDrawClipboard = FALSE;
> return 0;
> }
>
> @@ -248,11 +323,10 @@ winClipboardWindowProc (HWND hwnd, UINT
> if (!IsClipboardFormatAvailable (CF_TEXT)
> && !IsClipboardFormatAvailable (CF_UNICODETEXT))
> {
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> +
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "Clipboard does not contain CF_TEXT nor "
> "CF_UNICODETEXT.\n");
> -#endif
>
> /*
> * We need to make sure that the X Server has processed
> @@ -264,17 +338,15 @@ winClipboardWindowProc (HWND hwnd, UINT
> iReturn = XGetSelectionOwner (pDisplay, XA_PRIMARY);
> if (iReturn == g_iClipboardWindow)
> {
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "PRIMARY selection is owned by us.\n");
> -#endif
> XSetSelectionOwner (pDisplay,
> XA_PRIMARY,
> None,
> CurrentTime);
> }
> else if (BadWindow == iReturn || BadAtom == iReturn)
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winErrorFVerb (1, "winClipboardWindowProc - WM_DRAWCLIPBOARD -
> "
> "XGetSelection failed for PRIMARY: %d\n", iReturn);
>
> /* Release CLIPBOARD selection if owned */
> @@ -284,10 +356,8 @@ winClipboardWindowProc (HWND hwnd, UINT
> False));
> if (iReturn == g_iClipboardWindow)
> {
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "CLIPBOARD selection is owned by us.\n");
> -#endif
> XSetSelectionOwner (pDisplay,
> XInternAtom (pDisplay,
> "CLIPBOARD",
> @@ -296,9 +366,11 @@ winClipboardWindowProc (HWND hwnd, UINT
> CurrentTime);
> }
> else if (BadWindow == iReturn || BadAtom == iReturn)
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winErrorFVerb (1, "winClipboardWindowProc - WM_DRAWCLIPBOARD -
> "
> "XGetSelection failed for CLIPBOARD: %d\n", iReturn);
>
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD: Exit\n");
> + s_fProcessingDrawClipboard = FALSE;
> return 0;
> }
>
> @@ -309,16 +381,14 @@ winClipboardWindowProc (HWND hwnd, UINT
> CurrentTime);
> if (iReturn == BadAtom || iReturn == BadWindow)
> {
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winErrorFVerb (1, "winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "Could not reassert ownership of PRIMARY\n");
> }
> -#if 0
> else
> {
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "Reasserted ownership of PRIMARY\n");
> }
> -#endif
>
> /* Reassert ownership of the CLIPBOARD */
> iReturn = XSetSelectionOwner (pDisplay,
> @@ -329,20 +399,21 @@ winClipboardWindowProc (HWND hwnd, UINT
> CurrentTime);
> if (iReturn == BadAtom || iReturn == BadWindow)
> {
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winErrorFVerb (1, "winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "Could not reassert ownership of CLIPBOARD\n");
> }
> -#if 0
> else
> {
> - ErrorF ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD - "
> "Reasserted ownership of CLIPBOARD\n");
> }
> -#endif
>
> /* Flush the pending SetSelectionOwner event now */
> XFlush (pDisplay);
> +
> + s_fProcessingDrawClipboard = FALSE;
> }
> + winDebug ("winClipboardWindowProc - WM_DRAWCLIPBOARD: Exit\n");
> return 0;
>
>
> @@ -368,9 +439,7 @@ winClipboardWindowProc (HWND hwnd, UINT
> Window iWindow = g_iClipboardWindow;
> Bool fConvertToUnicode;
>
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_RENDER*FORMAT - Hello.\n");
> -#endif
> + winDebug ("winClipboardWindowProc - WM_RENDER*FORMAT - Hello.\n");
>
> /* Flag whether to convert to Unicode or not */
> if (message == WM_RENDERALLFORMATS)
> @@ -389,7 +458,7 @@ winClipboardWindowProc (HWND hwnd, UINT
> CurrentTime);
> if (iReturn == BadAtom || iReturn == BadWindow)
> {
> - ErrorF ("winClipboardWindowProc - WM_RENDER*FORMAT - "
> + winErrorFVerb (1, "winClipboardWindowProc - WM_RENDER*FORMAT - "
> "XConvertSelection () failed\n");
> break;
> }
> @@ -407,7 +476,7 @@ winClipboardWindowProc (HWND hwnd, UINT
>
> if (!OpenClipboard (hwnd))
> {
> - ErrorF ("winClipboardWindowProc - WM_RENDER*FORMATS - "
> + winErrorFVerb (1, "winClipboardWindowProc -
> WM_RENDER*FORMATS - "
> "OpenClipboard () failed: %08x\n",
> GetLastError ());
> break;
> @@ -415,7 +484,7 @@ winClipboardWindowProc (HWND hwnd, UINT
>
> if (!EmptyClipboard ())
> {
> - ErrorF ("winClipboardWindowProc - WM_RENDER*FORMATS - "
> + winErrorFVerb (1, "winClipboardWindowProc -
> WM_RENDER*FORMATS - "
> "EmptyClipboard () failed: %08x\n",
> GetLastError ());
> break;
> @@ -465,16 +534,14 @@ winClipboardWindowProc (HWND hwnd, UINT
>
> if (!CloseClipboard ())
> {
> - ErrorF ("winClipboardWindowProc - WM_RENDERALLFORMATS - "
> + winErrorFVerb (1, "winClipboardWindowProc -
> WM_RENDERALLFORMATS - "
> "CloseClipboard () failed: %08x\n",
> GetLastError ());
> break;
> }
> }
>
> -#if 0
> - ErrorF ("winClipboardWindowProc - WM_RENDER*FORMAT - Returning.\n");
> -#endif
> + winDebug ("winClipboardWindowProc - WM_RENDER*FORMAT -
> Returning.\n");
> return 0;
> }
> }
>
>
>
>
More information about the Cygwin-xfree
mailing list