Index: /trunk/src/VBox/HostServices/SharedClipboard/VBoxClipboard.h =================================================================== --- /trunk/src/VBox/HostServices/SharedClipboard/VBoxClipboard.h (revision 15897) +++ /trunk/src/VBox/HostServices/SharedClipboard/VBoxClipboard.h (revision 15898) @@ -25,9 +25,4 @@ #define LOG_GROUP LOG_GROUP_HGCM #include - -enum { - /** The number of milliseconds before the clipboard times out. */ - CLIPBOARDTIMEOUT = 2000 -}; struct _VBOXCLIPBOARDCONTEXT; Index: /trunk/src/VBox/HostServices/SharedClipboard/x11.cpp =================================================================== --- /trunk/src/VBox/HostServices/SharedClipboard/x11.cpp (revision 15897) +++ /trunk/src/VBox/HostServices/SharedClipboard/x11.cpp (revision 15898) @@ -194,6 +194,19 @@ LogFlowFunc(("u32Format=%02X\n", u32Format)); + /* Assert that no other transfer is in process (requests are serialised) + * and that the last transfer cleaned up properly. */ + AssertLogRelReturn( pCtx->pClient->data.pv == NULL + && pCtx->pClient->data.cb == 0 + && pCtx->pClient->data.u32Format == 0, + VERR_WRONG_ORDER + ); + /* No one else (X11 or VBox) should currently be waiting. The first because + * requests from X11 are serialised and the second because VBox previously + * grabbed the clipboard, so it should not be waiting for data from us. */ + AssertLogRelReturn (ASMAtomicCmpXchgU32(&pCtx->waiter, 1, 0), VERR_DEADLOCK); if (pClient == 0) { + /* This can legitimately happen if we disconnect during a request for + * data from X11. */ LogFunc(("host requested guest clipboard data after guest had disconnected.\n")); pCtx->guestFormats = 0; @@ -201,27 +214,16 @@ return VERR_TIMEOUT; } - if (!( pCtx->pClient->data.pv == NULL - && pCtx->pClient->data.cb == 0 - && pCtx->pClient->data.u32Format == 0)) - { - LogRel(("vboxClipboardReadDataFromClient: a guest to host clipboard transfer has been requested, but another is in progress, or has not cleaned up properly.\n")); - AssertMsgFailed(("A guest to host clipboard transfer has been requested, but another is in progress, or has not cleaned up properly.\n")); - } - - /* Only one of the guest and the host should be waiting at any one time */ - if (!ASMAtomicCmpXchgU32(&pCtx->waiter, 1, 0)) - { - LogRel(("vboxClipboardReadDataFromClient: deadlock situation - the host and the guest are both waiting for data from the other.\n")); - return VERR_DEADLOCK; - } /* Request data from the guest */ vboxSvcClipboardReportMsg (pCtx->pClient, VBOX_SHARED_CLIPBOARD_HOST_MSG_READ_DATA, u32Format); /* Which will signal us when it is ready. */ - if (RTSemEventWait(pCtx->waitForData, CLIPBOARDTIMEOUT) != VINF_SUCCESS) - { - LogRel(("vboxClipboardReadDataFromClient: vboxSvcClipboardReportMsg failed to complete within %d milliseconds\n", CLIPBOARDTIMEOUT)); + int rc = RTSemEventWait(pCtx->waitForData, RT_INDEFINITE_WAIT); + AssertLogRelRCSuccess(rc); + if (RT_FAILURE(rc)) + { + /* I believe this should not happen. Wait until the assertions arrive + * to prove the contrary. */ pCtx->guestFormats = 0; pCtx->waiter = 0; - return VERR_TIMEOUT; + return rc; } pCtx->waiter = 0; @@ -684,6 +686,4 @@ XtAppMainLoop(g_ctx.appContext); g_ctx.formatList.clear(); - RTSemEventDestroy(g_ctx.waitForData); - RTSemMutexDestroy(g_ctx.asyncMutex); LogRel(("Shared clipboard: host clipboard thread terminated successfully\n")); return VINF_SUCCESS; @@ -848,4 +848,8 @@ LogRel(("vboxClipboardDestroy: shutting down host clipboard\n")); + /* Drop the reference to the client, in case it is still there. This will + * cause any outstanding clipboard data requests from X11 to fail + * immediately. */ + g_ctx.pClient = NULL; /* Set the termination flag. */ XtAppSetExitFlag(g_ctx.appContext); @@ -856,4 +860,12 @@ XSendEvent(XtDisplay(g_ctx.widget), XtWindow(g_ctx.widget), false, 0, &ev); XFlush(XtDisplay(g_ctx.widget)); + if (g_ctx.eOwner == GUEST) + /* X11 may be waiting for data from VBox. At this point it is no + * longer going to arrive, and we must release it to allow the event + * loop to terminate. In this case the buffer where VBox would have + * written the clipboard data will still be empty and we will just + * return "no data" to X11. Any subsequent attempts to get the data + * from VBox will fail immediately as the client reference is gone. */ + RTSemEventSignal(g_ctx.waitForData); do { @@ -863,5 +875,17 @@ } while ((VERR_TIMEOUT == rc) && (count < 300)); if (RT_SUCCESS(rc)) + { + /* + * No one should be waiting on this by now. Justification: + * - Case 1: VBox is waiting for data from X11: + * Not possible, as it would be waiting on this thread. + * - Case 2: X11 is waiting for data from VBox: + * Not possible, as we checked that the X11 event thread exited + * successfully. + */ + RTSemEventDestroy(g_ctx.waitForData); + RTSemMutexDestroy(g_ctx.asyncMutex); AssertRC(rcThread); + } else LogRel(("vboxClipboardDestroy: rc=%Rrc\n", rc)); @@ -889,9 +913,5 @@ /* Only one client is supported for now */ - if (g_ctx.pClient != 0) - { - LogRel(("vboxClipboardConnect: attempted to connect, but a client appears to be already running.\n")); - AssertReturn(g_ctx.pClient == 0, VERR_NOT_SUPPORTED); - } + AssertLogRelReturn(g_ctx.pClient == 0, VERR_NOT_SUPPORTED); pClient->pCtx = &g_ctx; @@ -1045,5 +1065,5 @@ /* If vboxClipboardReadDataFromVBox fails then pClient may be invalid */ LogRelFunc (("vboxClipboardReadDataFromVBox returned %Rrc%s\n", rc, - RT_SUCCESS(rc) ? ", g_ctx.pClient->data.cb == 0" : "")); + RT_SUCCESS(rc) ? ", g_ctx.pClient->data.cb == 0" : "")); vboxClipboardEmptyGuestBuffer(); return false; @@ -1120,5 +1140,5 @@ /* If vboxClipboardReadDataFromVBox fails then pClient may be invalid */ LogRelFunc (("vboxClipboardReadDataFromVBox returned %Rrc%s\n", rc, - RT_SUCCESS(rc) ? ", g_ctx.pClient->data.cb == 0" : "")); + RT_SUCCESS(rc) ? ", g_ctx.pClient->data.cb == 0" : "")); vboxClipboardEmptyGuestBuffer(); return false; @@ -1319,4 +1339,7 @@ LogFlowFunc(("\n")); + /* Drop requests that we receive too late. */ + if (g_ctx.eOwner != GUEST) + return false; if ( (*atomSelection != g_ctx.atomClipboard) && (*atomSelection != g_ctx.atomPrimary) @@ -1475,10 +1498,8 @@ return VERR_NO_DATA; /* The guest thinks we have data and we don't */ } - /* Only one of the host and the guest should ever be waiting. */ - if (!ASMAtomicCmpXchgU32(&g_ctx.waiter, 1, 0)) - { - LogRel(("vboxClipboardReadData: detected a deadlock situation - the host and the guest are waiting for each other.\n")); - return VERR_DEADLOCK; - } + /* No one else (VBox or X11) should currently be waiting. The first because + * requests from VBox are serialised and the second because X11 previously + * grabbed the clipboard, so it should not be waiting for data from us. */ + AssertLogRelReturn (ASMAtomicCmpXchgU32(&g_ctx.waiter, 1, 0), VERR_DEADLOCK); g_ctx.requestHostFormat = g_ctx.hostTextFormat; g_ctx.requestBuffer = pv; @@ -1492,27 +1513,12 @@ vboxClipboardGetDataFromX11, reinterpret_cast(g_ctx.pClient), CurrentTime); - /* When the data arrives, the vboxClipboardGetProc callback will be called. The + /* When the data arrives, the vboxClipboardGetDataFromX11 callback will be called. The callback will signal the event semaphore when it has processed the data for us. */ -#ifdef RT_OS_SOLARIS - /* - * Trusted Xorg requires a bigger timeout. - */ - unsigned long cTimeoutMillies = CLIPBOARDTIMEOUT; - if (is_system_labeled()) - cTimeoutMillies = 1000 * 120; - - if (RTSemEventWait(g_ctx.waitForData, cTimeoutMillies) != VINF_SUCCESS) -#else - if (RTSemEventWait(g_ctx.waitForData, CLIPBOARDTIMEOUT) != VINF_SUCCESS) -#endif + int rc = RTSemEventWait(g_ctx.waitForData, RT_INDEFINITE_WAIT); + if (RT_FAILURE(rc)) { - /* No need to polute the release log for this. */ - // LogRel(("vboxClipboardReadDataFromClient: XtGetSelectionValue failed to complete within %d milliseconds\n", CLIPBOARDTIMEOUT)); - /* A time out can legitimately occur if a client is temporarily too busy to answer fast */ - // g_ctx.hostTextFormat = INVALID; - // g_ctx.hostBitmapFormat = INVALID; g_ctx.waiter = 0; - return VERR_TIMEOUT; + return rc; } g_ctx.waiter = 0; @@ -1542,14 +1548,9 @@ LogFlowFunc (("called\n")); - /* - * The guest returns data that was requested in the WM_RENDERFORMAT handler. - */ - if (!( pClient->data.pv == NULL - && pClient->data.cb == 0 - && pClient->data.u32Format == 0)) - { - LogRel(("vboxClipboardWriteData: clipboard data has arrived from the guest, but another transfer is in process or has not cleaned up properly.\n")); - AssertMsgFailed(("vboxClipboardWriteData: clipboard data has arrived from the guest, but another transfer is in process or has not cleaned up properly.\n")); - } + /* Assert that no other transfer is in process (requests are serialised) + * or has not cleaned up properly. */ + AssertLogRelReturnVoid ( pClient->data.pv == NULL + && pClient->data.cb == 0 + && pClient->data.u32Format == 0); if (cb > 0) @@ -1567,2 +1568,3 @@ RTSemEventSignal(g_ctx.waitForData); } +