[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [krita/rempt/impex-refactoring] libs/image: Fix race condition in KisMergeWalker::startTripWithMask
From:       Frederik Gladhorn <gladhorn () kde ! org>
Date:       2016-11-16 16:24:14
Message-ID: E1c730Q-0000tO-Ew () code ! kde ! org
[Download RAW message or body]

Git commit 2994b9824edfec32b4b5cb17c4188394b226db24 by Frederik Gladhorn.
Committed on 16/11/2016 at 16:24.
Pushed by gladhorn into branch 'rempt/impex-refactoring'.

Fix race condition in KisMergeWalker::startTripWithMask

Summary:
The parent pointer could become null right after the check.
Making it safe is easy, just hold on to the pointer for the duration
of the function.

Reviewers: #krita

Differential Revision: https://phabricator.kde.org/D3388

M  +13   -16   libs/image/kis_merge_walker.cc

http://commits.kde.org/krita/2994b9824edfec32b4b5cb17c4188394b226db24

diff --git a/libs/image/kis_merge_walker.cc b/libs/image/kis_merge_walker.cc
index ef726ab..c0cff45 100644
--- a/libs/image/kis_merge_walker.cc
+++ b/libs/image/kis_merge_walker.cc
@@ -53,27 +53,24 @@ void KisMergeWalker::startTrip(KisProjectionLeafSP startLeaf)
 
 void KisMergeWalker::startTripWithMask(KisProjectionLeafSP filthyMask)
 {
-    if (!filthyMask->parent()) {
-        /**
-         * Under very rare circumstances it may happen that the update
-         * queue will contain a job pointing to a node that has
-         * already been deleted from the image (direclty or by undo
-         * command). If it happens to a layer then the walker will
-         * handle it as usual by building a trivial graph pointing to
-         * nowhere, but when it happens to a mask... not. Because the
-         * mask is always expected to have a parent layer to process.
-         *
-         * So just handle it here separately.
-         */
-
+    /**
+     * Under very rare circumstances it may happen that the update
+     * queue will contain a job pointing to a node that has
+     * already been deleted from the image (direclty or by undo
+     * command). If it happens to a layer then the walker will
+     * handle it as usual by building a trivial graph pointing to
+     * nowhere, but when it happens to a mask... not. Because the
+     * mask is always expected to have a parent layer to process.
+     *
+     * So just handle it here separately.
+     */
+    KisProjectionLeafSP parentLayer = filthyMask->parent();
+    if (!parentLayer) {
         return;
     }
 
     adjustMasksChangeRect(filthyMask);
 
-    KisProjectionLeafSP parentLayer = filthyMask->parent();
-    Q_ASSERT(parentLayer);
-
     KisProjectionLeafSP nextLeaf = parentLayer->nextSibling();
     KisProjectionLeafSP prevLeaf = parentLayer->prevSibling();
 
[prev in list] [next in list] [prev in thread] [next in thread]