From 430aa087147ef81a196330691e5e4a0c2e0bbeb9 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Fri, 2 Oct 2020 08:56:06 +0300 Subject: [PATCH] Improve readability of code that destroys frame callback resources If a frame callback resource is destroyed, it will unregister itself from corresponding lists in current, pending, and cached state. However, this means that we need to be super duper careful when the compositor wants to destroy all frame callbacks. We need to make a copy of a frameCallbacks list; otherwise a call to removeOne() will invalidate iterators and the compositor may crash. Currently, that copy is made implicitly. Some people may see that code and add qAsConst() without realizing the consequences it will lead to. This change improves the readability of that code by making explicit copies of frameCallbacks in code that shuts down SurfaceInterface. --- src/wayland/surface_interface.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index af3f857eb3..b0913a5ebc 100644 --- a/src/wayland/surface_interface.cpp +++ b/src/wayland/surface_interface.cpp @@ -54,15 +54,22 @@ SurfaceInterfacePrivate::SurfaceInterfacePrivate(SurfaceInterface *q) SurfaceInterfacePrivate::~SurfaceInterfacePrivate() { - for (KWaylandFrameCallback *frameCallback : current.frameCallbacks) { + // Need a copy to avoid hitting invalidated iterators in the for loop. + const QList currentFrameCallbacks = current.frameCallbacks; + for (KWaylandFrameCallback *frameCallback : currentFrameCallbacks) { frameCallback->destroy(); } - for (KWaylandFrameCallback *frameCallback : pending.frameCallbacks) { + + const QList pendingFrameCallbacks = pending.frameCallbacks; + for (KWaylandFrameCallback *frameCallback : pendingFrameCallbacks) { frameCallback->destroy(); } - for (KWaylandFrameCallback *frameCallback : cached.frameCallbacks) { + + const QList cachedFrameCallbacks = cached.frameCallbacks; + for (KWaylandFrameCallback *frameCallback : cachedFrameCallbacks) { frameCallback->destroy(); } + if (current.buffer) { current.buffer->unref(); }