MirSurfaceItem::dropPendingBuffers contains a potentially infinite loop

Bug #1477430 reported by Daniel van Vugt
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
qtmir (Ubuntu)
Fix Released
Medium
Daniel van Vugt

Bug Description

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 even 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 of unblocking the client. No loop required.

Related branches

description: updated
description: updated
Changed in qtmir:
assignee: nobody → Daniel van Vugt (vanvugt)
status: New → In Progress
Changed in qtmir:
importance: Undecided → Medium
Changed in qtmir (Ubuntu):
importance: Undecided → Medium
Changed in qtmir:
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qtmir - 0.4.5+15.10.20150817-0ubuntu1

---------------
qtmir (0.4.5+15.10.20150817-0ubuntu1) wily; urgency=medium

  *

qtmir (0.4.5+15.10.20150812.1-0ubuntu1) vivid; urgency=medium

  [ Daniel van Vugt ]
  * MirSurfaceItem: Remove an unnecessary and potentially infinite loop
    (LP: #1477430) (LP: #1477430)

  [ Gerry Boland ]
  * Standardize licences to LGPLv3, update years, remove authors (LP:
    #1483664)
  * authorizeSession incorrectly edits desktopFilePath supplied by
    desktop_file_hint (LP: #1483225)

 -- CI Train Bot <email address hidden> Mon, 17 Aug 2015 19:28:26 +0000

Changed in qtmir (Ubuntu):
status: New → Fix Released
Gerry Boland (gerboland)
Changed in qtmir:
status: Fix Committed → Fix Released
Michał Sawicz (saviq)
Changed in qtmir (Ubuntu):
assignee: nobody → Daniel van Vugt (vanvugt)
no longer affects: qtmir
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.