Comment 0 for bug 1477430

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

MirSurfaceItem::dropPendingBuffers contains a potentially infinite/indefinite loop:

void MirSurfaceItem::dropPendingBuffers()
{
    QMutexLocker locker(&m_mutex);

    const void* const userId = (void*)123; // TODO: Multimonitor support

    while (m_surface->buffers_ready_for_compositor(userId) > 0) {
        // The line below looks like an innocent, effect-less, getter. But as this
        // method returns a unique_pointer, not holding its reference causes the
        // buffer to be destroyed/released straight away.
        m_surface->compositor_snapshot(userId)->buffer();
        qCDebug(QTMIR_SURFACES) << "MirSurfaceItem::dropPendingBuffers()"
            << "surface =" << this
            << "buffer dropped."
            << m_surface->buffers_ready_for_compositor(userId)
            << "left.";
    }
}

The issue is the value of buffers_ready_for_compositor() could theoretically grow just as quickly as you're consuming the buffers. So that loop has no definite number of iterations before exiting.

The simple solution is to check buffers_ready_for_compositor() once, save the result and use a for loop up to that limit. However event that's an overkill. If you look at the purpose of dropPendingBuffers, it's a misnomer and it should really be "dropPendingBuffer" as it only really needs to consume a single frame to achieve its goal; no loop required.