2015-07-23 07:41:55 |
Daniel van Vugt |
bug |
|
|
added bug |
2015-07-23 07:42:36 |
Daniel van Vugt |
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 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. |
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; no loop required. |
|
2015-07-23 07:43:25 |
Daniel van Vugt |
bug task added |
|
qtmir (Ubuntu) |
|
2015-07-23 07:44:20 |
Daniel van Vugt |
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; no loop required. |
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. |
|
2015-07-23 14:51:36 |
Daniel d'Andrada |
bug |
|
|
added subscriber Daniel d'Andrada |
2015-07-24 07:31:48 |
Daniel van Vugt |
qtmir: assignee |
|
Daniel van Vugt (vanvugt) |
|
2015-07-24 07:31:52 |
Daniel van Vugt |
qtmir: status |
New |
In Progress |
|
2015-07-24 07:50:49 |
Daniel van Vugt |
branch linked |
|
lp:~vanvugt/qtmir/smoother-mirsurfaceitem |
|
2015-07-28 08:45:46 |
Daniel van Vugt |
branch linked |
|
lp:~vanvugt/qtmir/fix-1477430 |
|
2015-07-28 08:47:54 |
Daniel van Vugt |
qtmir: importance |
Undecided |
Medium |
|
2015-07-28 08:47:57 |
Daniel van Vugt |
qtmir (Ubuntu): importance |
Undecided |
Medium |
|
2015-08-21 03:30:15 |
Daniel van Vugt |
qtmir: status |
In Progress |
Fix Committed |
|
2015-08-24 05:06:13 |
Launchpad Janitor |
qtmir (Ubuntu): status |
New |
Fix Released |
|
2015-09-02 14:57:51 |
Gerry Boland |
qtmir: status |
Fix Committed |
Fix Released |
|
2017-03-13 17:35:15 |
Michał Sawicz |
qtmir (Ubuntu): assignee |
|
Daniel van Vugt (vanvugt) |
|
2017-03-13 17:35:23 |
Michał Sawicz |
bug task deleted |
qtmir |
|
|