hw/xwin: Improve reliability of clipboard X->Windows pastes

Sometimes, particularly with large clipboard pastes to Windows, we could end up
waiting for the timeout to expire, rather than pasting the data.

Various changes to improve reliability:

1. Use XFlush() not XSync() in winProcessXEventsTimeout().

It makes no sense to ensure we have received replies to outstanding requests if
we are going to wait for them using select()

2. Add XFlush() to winClipboardProc()

Make sure we have sent any requests before we wait using select()

3. Don't use FD_ISSET() to check which fd is ready

This looks like a Cygwin select() bug in that it sometimes returns 0 with an
empty fd set before the timeout expires, but a fd appears to be ready.

Add select() return value to debug output when we are warning that this has
happened.

4. Drain event queues before entering select()

Unconditionally drain event queues before entering select().  This seems to be
the recommended way of writing select() and X event processing loops.

winClipboardFlushXEvents() checks using XPending(), and
winClipboardFlushWindowsMessageQueue() checks using PeekMessage() so this is
safe against blocking, but means that may not need to enter select() at all
sometimes.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Reviewed-by: Colin Harrison <colin.harrison@virgin.net>

2 files changed, 41 insertions, 43 deletions

diff --git a/hw/xwin/winclipboard/thread.c b/hw/xwin/winclipboard/thread.c
index be053574d..786b88972 100644
--- a/hw/xwin/winclipboard/thread.c
+++ b/hw/xwin/winclipboard/thread.c
@@ -255,22 +255,25 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay)
         }
     }
 
-    /* Pre-flush X events */
-    /*
-     * NOTE: Apparently you'll freeze if you don't do this,
-     *       because there may be events in local data structures
-     *       already.
-     */
     data.fUseUnicode = fUseUnicode;
-    winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms);
-
-    /* Pre-flush Windows messages */
-    if (!winClipboardFlushWindowsMessageQueue(hwnd)) {
-        ErrorF("winClipboardProc - winClipboardFlushWindowsMessageQueue failed\n");
-    }
 
-    /* Loop for X events */
+    /* Loop for events */
     while (1) {
+
+        /* Process X events */
+        winClipboardFlushXEvents(hwnd,
+                                 iWindow, pDisplay, &data, &atoms);
+
+        /* Process Windows messages */
+        if (!winClipboardFlushWindowsMessageQueue(hwnd)) {
+          ErrorF("winClipboardProc - winClipboardFlushWindowsMessageQueue trapped "
+                       "WM_QUIT message, exiting main loop.\n");
+          break;
+        }
+
+        /* We need to ensure that all pending requests are sent */
+        XFlush(pDisplay);
+
         /* Setup the file descriptor set */
         /*
          * NOTE: You have to do this before every call to select
@@ -317,10 +320,9 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay)
             break;
         }
 
-        /* Branch on which descriptor became active */
         if (FD_ISSET(iConnectionNumber, &fdsRead)) {
-            /* Process X events */
-            winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms);
+            winDebug
+                ("winClipboardProc - X connection ready, pumping X event queue\n");
         }
 
 #ifdef HAS_DEVWINDOWS
@@ -330,14 +332,16 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay)
         if (1)
 #endif
         {
-            /* Process Windows messages */
-            if (!winClipboardFlushWindowsMessageQueue(hwnd)) {
-                ErrorF("winClipboardProc - "
-                       "winClipboardFlushWindowsMessageQueue trapped "
-                       "WM_QUIT message, exiting main loop.\n");
-                break;
-            }
+            winDebug
+                ("winClipboardProc - /dev/windows ready, pumping Windows message queue\n");
         }
+
+#ifdef HAS_DEVWINDOWS
+        if (!(FD_ISSET(iConnectionNumber, &fdsRead)) &&
+            !(FD_ISSET(fdMessageQueue, &fdsRead))) {
+            winDebug("winClipboardProc - Spurious wake, select() returned %d\n", iReturn);
+        }
+#endif
     }
 
  winClipboardProc_Exit:
diff --git a/hw/xwin/winclipboard/wndproc.c b/hw/xwin/winclipboard/wndproc.c
index 23a0ac9bf..b63a1dc5c 100644
--- a/hw/xwin/winclipboard/wndproc.c
+++ b/hw/xwin/winclipboard/wndproc.c
@@ -83,8 +83,18 @@ winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay,
         fd_set fdsRead;
         long remainingTime;
 
-        /* We need to ensure that all pending events are processed */
-        XSync(pDisplay, FALSE);
+        /* Process X events */
+        iReturn = winClipboardFlushXEvents(hwnd, iWindow, pDisplay, data, atoms);
+
+        winDebug("winProcessXEventsTimeout () - winClipboardFlushXEvents returned %d\n", iReturn);
+
+        if ((WIN_XEVENTS_NOTIFY_DATA == iReturn) || (WIN_XEVENTS_NOTIFY_TARGETS == iReturn) || (WIN_XEVENTS_FAILED == iReturn)) {
+          /* Bail out */
+          return iReturn;
+        }
+
+        /* We need to ensure that all pending requests are sent */
+        XFlush(pDisplay);
 
         /* Setup the file descriptor set */
         FD_ZERO(&fdsRead);
@@ -113,24 +123,8 @@ winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay,
             break;
         }
 
-        /* Branch on which descriptor became active */
-        if (FD_ISSET(iConnNumber, &fdsRead)) {
-            /* Process X events */
-            /* Exit when we see that server is shutting down */
-            iReturn = winClipboardFlushXEvents(hwnd,
-                                               iWindow, pDisplay, data, atoms);
-
-            winDebug
-                ("winProcessXEventsTimeout () - winClipboardFlushXEvents returned %d\n",
-                 iReturn);
-
-            if ((WIN_XEVENTS_NOTIFY_DATA == iReturn) || (WIN_XEVENTS_NOTIFY_TARGETS == iReturn) || (WIN_XEVENTS_FAILED == iReturn)) {
-                /* Bail out */
-                return iReturn;
-            }
-        }
-        else {
-            winDebug("winProcessXEventsTimeout - Spurious wake\n");
+        if (!FD_ISSET(iConnNumber, &fdsRead)) {
+            winDebug("winProcessXEventsTimeout - Spurious wake, select() returned %d\n", iReturn);
         }
     }