From 8f0530c50755cbd860c92039ac22396d6ed56a24 Mon Sep 17 00:00:00 2001 From: Nye Liu Date: Fri, 11 Sep 2015 10:42:01 -0700 Subject: [PATCH] Add retry to CondVarBase wait(), make sure Stopwatch is started on construction (Issue #4735) * ArchMultithreadPosix::waitCondVar() returns every 100ms, so retry until we hit timeout. * Stopwatch constructor should be called with "false" (not "true") to make sure Stopwatch is actually running when instantiated. --- src/lib/client/Client.cpp | 9 +++++++-- src/lib/mt/CondVar.cpp | 14 ++++++++------ src/lib/platform/XWindowsClipboard.cpp | 5 +++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/lib/client/Client.cpp b/src/lib/client/Client.cpp index 41b1df24..33048807 100644 --- a/src/lib/client/Client.cpp +++ b/src/lib/client/Client.cpp @@ -283,13 +283,15 @@ Client::leave() &Client::sendClipboardThread, NULL)); // Bug #4735 - we can't leave() until fillClipboard()s all finish - Stopwatch timer(true); + Stopwatch timer(false); m_mutex->lock(); while (!m_condData) { if (!m_condVar->wait(timer, 0.5)) { - LOG((CLOG_DEBUG "timed out waiting for clipboard fill")); + LOG((CLOG_WARN "timed out %fs waiting for clipboard fill", + (double) timer.getTime())); break; } + LOG((CLOG_DEBUG1 "leave %fs elapsed", (double) timer.getTime())); } m_mutex->unlock(); @@ -789,6 +791,7 @@ Client::onFileRecieveCompleted() void Client::sendClipboardThread(void * data) { + Stopwatch timer(false); Clipboard clipboard[kClipboardEnd]; // fill clipboards that we own and that have changed for (ClipboardID id = 0; id < kClipboardEnd; ++id) { @@ -796,6 +799,7 @@ Client::sendClipboardThread(void * data) fillClipboard(id, &clipboard[id]); } } + LOG((CLOG_DEBUG1 "fill took %fs, signaling", (double) timer.getTime())); // signal that fill is done m_mutex->lock(); @@ -811,6 +815,7 @@ Client::sendClipboardThread(void * data) } m_sendClipboardThread = NULL; + LOG((CLOG_DEBUG1 "send took %fs", (double) timer.getTime())); } void diff --git a/src/lib/mt/CondVar.cpp b/src/lib/mt/CondVar.cpp index 2ec18e46..caa12210 100644 --- a/src/lib/mt/CondVar.cpp +++ b/src/lib/mt/CondVar.cpp @@ -63,13 +63,15 @@ CondVarBase::broadcast() bool CondVarBase::wait(Stopwatch& timer, double timeout) const { - // check timeout against timer - if (timeout >= 0.0) { - timeout -= timer.getTime(); - if (timeout < 0.0) - return false; + double remain = timeout-timer.getTime(); + // Some ARCH wait()s return prematurely, retry until really timed out + // In particular, ArchMultithreadPosix::waitCondVar() returns every 100ms + while (remain >= 0.0) { + if (wait(remain)) + return true; + remain = timeout - timer.getTime(); } - return wait(timeout); + return false; } bool diff --git a/src/lib/platform/XWindowsClipboard.cpp b/src/lib/platform/XWindowsClipboard.cpp index 7db899f1..ef587739 100644 --- a/src/lib/platform/XWindowsClipboard.cpp +++ b/src/lib/platform/XWindowsClipboard.cpp @@ -513,6 +513,7 @@ XWindowsClipboard::icccmFillCache() LOG((CLOG_DEBUG1 "selection doesn't support TARGETS")); data = ""; XWindowsUtil::appendAtomData(data, XA_STRING); + // return; // NTL } XWindowsUtil::convertAtomProperty(data); @@ -1317,7 +1318,7 @@ XWindowsClipboard::CICCCMGetClipboard::readClipboard(Display* display, // by badly behaved selection owners. XEvent xevent; std::vector events; - Stopwatch timeout(true); + Stopwatch timeout(false); // timer not stopped, not triggered static const double s_timeout = 0.25; // FIXME -- is this too short? bool noWait = false; while (!m_done && !m_failed) { @@ -1361,7 +1362,7 @@ XWindowsClipboard::CICCCMGetClipboard::readClipboard(Display* display, XSelectInput(display, m_requestor, attr.your_event_mask); // return success or failure - LOG((CLOG_DEBUG1 "request %s", m_failed ? "failed" : "succeeded")); + LOG((CLOG_DEBUG1 "request %s after %fs", m_failed ? "failed" : "succeeded", timeout.getTime())); return !m_failed; }