xcb: fix thread synchronization in QXcbEventQueue::waitForNewEvents() again

This patch amends a41701904e

If peeking into the event queue looking for a clipboard event fails,
QXcbClipboard::waitForClipboardEvent() calls queue->peek for the second
time to "process other clipboard events, since someone is probably
requesting data from us". QXcbEventQueue::peek() in turn calls
QXcbEventQueue::flushBufferedEvents(). This second flushing can acquire
a waited-for clipboard event. The issue was that the code in
waitForNewEvents() ignored this possibility and assumed that there were
no clipboard events before or at its current m_flushedTail. If there
were no more events on the X11 connection after tailBeforeFlush,
the waitForNewEvents() in waitForClipboardEvent() blocked execution
for 5 seconds and eventually timed out.

The fix is to remember QXcbEventQueue::m_flushedTail just after looking
for and not finding a clipboard event in the queue. And then wait for
more events via QWaitCondition in waitForNewEvents() only if there were
no more events after the remembered m_flushedTail.

Fixes: QTBUG-75319
Pick-to: 5.15
Pick-to: 5.12
Change-Id: I4919c5b9b9227b3a8a29a11e7094f97960b3a121
Reviewed-by: Gatis Paeglis <gatis.paeglis@qt.io>
This commit is contained in:
Igor Kushnir 2020-01-10 19:48:43 +02:00 committed by Gatis Paeglis
parent c6dde54e67
commit f2d22d5a51
3 changed files with 15 additions and 5 deletions

View File

@ -781,6 +781,12 @@ xcb_generic_event_t *QXcbClipboard::waitForClipboardEvent(xcb_window_t window, i
if (e) // found the waited for event
return e;
// It is safe to assume here that the pointed to node won't be re-used
// while we are holding the pointer to it. The nodes can be recycled
// only when they are dequeued, which is done only by
// QXcbConnection::processXcbEvents().
const QXcbEventNode *flushedTailNode = queue->flushedTail();
if (checkManager) {
auto reply = Q_XCB_REPLY(xcb_get_selection_owner, xcb_connection(), atom(QXcbAtom::CLIPBOARD_MANAGER));
if (!reply || reply->owner == XCB_NONE)
@ -806,7 +812,7 @@ xcb_generic_event_t *QXcbClipboard::waitForClipboardEvent(xcb_window_t window, i
const auto elapsed = timer.elapsed();
if (elapsed < clipboard_timeout)
queue->waitForNewEvents(clipboard_timeout - elapsed);
queue->waitForNewEvents(flushedTailNode, clipboard_timeout - elapsed);
} while (timer.elapsed() < clipboard_timeout);
return nullptr;

View File

@ -226,6 +226,8 @@ void QXcbEventQueue::run()
};
while (!m_closeConnectionDetected && (event = xcb_wait_for_event(connection))) {
// This lock can block only if there are users of waitForNewEvents().
// Currently only the clipboard implementation relies on it.
m_newEventsMutex.lock();
enqueueEvent(event);
while (!m_closeConnectionDetected && (event = xcb_poll_for_queued_event(connection)))
@ -350,12 +352,12 @@ bool QXcbEventQueue::peekEventQueue(PeekerCallback peeker, void *peekerData,
return result;
}
void QXcbEventQueue::waitForNewEvents(unsigned long time)
void QXcbEventQueue::waitForNewEvents(const QXcbEventNode *sinceFlushedTail,
unsigned long time)
{
QMutexLocker locker(&m_newEventsMutex);
QXcbEventNode *tailBeforeFlush = m_flushedTail;
flushBufferedEvents();
if (tailBeforeFlush != m_flushedTail)
if (sinceFlushedTail != m_flushedTail)
return;
m_newEventsCondition.wait(&m_newEventsMutex, time);
}

View File

@ -107,7 +107,9 @@ public:
bool peekEventQueue(PeekerCallback peeker, void *peekerData = nullptr,
PeekOptions option = PeekDefault, qint32 peekerId = -1);
void waitForNewEvents(unsigned long time = std::numeric_limits<unsigned long>::max());
const QXcbEventNode *flushedTail() const { return m_flushedTail; }
void waitForNewEvents(const QXcbEventNode *sinceFlushedTail,
unsigned long time = std::numeric_limits<unsigned long>::max());
private:
QXcbEventNode *qXcbEventNodeFactory(xcb_generic_event_t *event);