On 29/09/2010 15:05, Michel Hummel wrote:
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
I really appreciate your work on this set of patches, and I'm sorry it's taken
me so long to get around to looking at them.
I must confess that the size of each patch didn't help. You can never make
patches too small :-). 'git gui' and 'git rebase --interactive' are excellent
tools for splitting, merging and reordering patches.
I've merged most of it, with some tweaks and rearrangement into my latest
snapshot (code is at [1]), and it should be included in a 1.9.1-1 release I'll
be making shortly.
A few detailed comments:
I really want to keep fdMessageQueue under HAS_DEVWINDOWS for clarity and for
ease of sharing patches with XMing, so I had to rearrange things a bit to
permit that.
diff --git a/hw/xwin/winclipboardthread.c b/hw/xwin/winclipboardthread.c
index f85d8cf..5c8d0be 100644
--- a/hw/xwin/winclipboardthread.c
+++ b/hw/xwin/winclipboardthread.c
@@ -119,7 +119,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 */
@@ -143,7 +143,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)
{
These two failure modes are unlikely to get better when we restart the thread,
so I've tweaked these so they will be handled as a critical problem, causing
the X server to exit.
+ /* global clipboard variable reset */
+ g_fClipboardLaunched = FALSE;
+ g_fClipboardStarted = FALSE;
+ g_iClipboardWindow = None;
+ g_hwndClipboard = NULL;
+ g_fUnicodeClipboard = FALSE;
+ g_pClipboardDisplay = NULL;
Resetting g_fUnicodeClipboard seems unlikely to be right, as that will always
turn off unicode clipboard handling for all but the first invocation of the
thread.
diff --git a/hw/xwin/winclipboardthread.c b/hw/xwin/winclipboardthread.c
index 2d1ecf9..437f0fa 100644
--- a/hw/xwin/winclipboardthread.c
+++ b/hw/xwin/winclipboardthread.c
@@ -503,6 +503,16 @@ winClipboardErrorHandler (Display *pDisplay, XErrorEvent
*pErr)
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;
}
I left this bit out for the moment. This code doesn't seem to do what it says
it does. Checking for a BadWindow would be pErr->error_code == BadWindow (3).
I think checking pErr->request_code == 24 is checking the operation is
X_ConvertSelection (see /usr/include/X11/Xproto.h)
I'm not sure currently works correctly at all in multiwindow mode, as
XSetErrorHandler() also gets called by the internal WM thread, so this error
handler may not be the one installed.
Is there a reason why we can't work out which Xlib function is actually
failing BadWindow and handle it there?
Can you explain a bit more about the case which this helps with. Are you
xkill-ing the clipboard X window?
On another topic, I am still a bit concerned we have a potential race
condition with the two uses of winInitClipboard() during a server
regeneration. g_fClipboardLaunched is FALSE before it's called, and set
afterwards, but since these two calls happen in different threads, there's
nothing I can see preventing the thread getting started twice.
It might be that having the clipboard thread to exit after setting
g_fClipboardLaunched to FALSE if the server generation has changed creates the
needed ordering of events (if it doesn't already exist), but it would take
some staring at the code to be sure...
[1] http://cgit.freedesktop.org/~jturney/xserver/log/?h=snapshot
--
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/
