Bug 410572

Summary: Crash when using Bezier tool for a short path
Product: [Applications] krita Reporter: Karl Ove Hufthammer <karl>
Component: Tools/VectorAssignee: Dmitry Kazakov <dimula73>
Status: RESOLVED FIXED    
Severity: crash CC: daviddoc1405, griffinvalley
Priority: NOR    
Version: git master (please specify the git hash!)   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:

Description Karl Ove Hufthammer 2019-08-04 13:17:13 UTC
SUMMARY
When using the Bezier tool to draw and close a very short path, Krita crashes.

STEPS TO REPRODUCE
1. Create a new image.
2. Add and activate a verctor layer.
3. Select the Bezier tool.
4. Click once to start a Bezier path.
5. Move the pointer a little bit (just so the starting point is no longer red).
6. Double click.

OBSERVED RESULT
Krita crashes.

EXPECTED RESULT
Krita shouldn’t crash …

SOFTWARE/OS VERSIONS
Operating System: openSUSE Tumbleweed 20190730
KDE Plasma Version: 5.16.3
KDE Frameworks Version: 5.60.0
Qt Version: 5.13.0
Kernel Version: 5.2.3-1-default
OS Type: 64-bit
Comment 1 wolthera 2019-08-04 13:19:41 UTC
Yes, I got an asan backtrace here:
===================================
=================================================================
==1199==ERROR: AddressSanitizer: attempting double-free on 0x60400327a4d0 in thread T1039 (Thread (pooled)):
thr    #0 0x7ffff6ef87b8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8)
    #1 0x7fffeb37b055 in QMapDataBase::freeNodeAndRebalance(QMapNodeBase*) (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0x117055)
    #2 0x7fffe77939d8 in QMapData<KoShape*, KoRTree<KoShape*>::LeafNode*>::deleteNode(QMapNode<KoShape*, KoRTree<KoShape*>::LeafNode*>*) (/home/wolthera/krita/inst/lib/x86_64-linux-gnu/libkritaflake.so.19+0x2059d8)
    #3 0x7fffe778dfc7 in QMap<KoShape*, KoRTree<KoShape*>::LeafNode*>::remove(KoShape* const&) /usr/include/x86_64-linux-gnu/qt5/QtCore/qmap.h:948
    #4 0x7fffe778a71d in KoRTree<KoShape*>::remove(KoShape* const&) /home/wolthera/krita/src/libs/flake/KoRTree.h:461
    #5 0x7fffe777c91d in KoShapeManager::Private::updateTree() /home/wolthera/krita/src/libs/flake/KoShapeManager.cpp:82
    #6 0x7fffe777fceb in KoShapeManager::paint(QPainter&, KoViewConverter const&, bool) /home/wolthera/krita/src/libs/flake/KoShapeManager.cpp:264
    #7 0x7ffff1b825fe in KisShapeLayerCanvas::repaint() /home/wolthera/krita/src/libs/ui/flake/kis_shape_layer_canvas.cpp:326
    #8 0x7ffff1b85706 in KisRepaintShapeLayerLayerJob::run() (/home/wolthera/krita/inst/lib/x86_64-linux-gnu/libkritaui.so.19+0x2cae706)
    #9 0x7fffee396f16 in KisUpdateJobItem::run() /home/wolthera/krita/build/libs/image/kritaimage_autogen/EWIEGA46WW/../../../../../src/libs/image/kis_update_job_item.h:91
    #10 0x7fffeb3103e1  (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0xac3e1)
    #11 0x7fffeb30bc71  (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0xa7c71)
    #12 0x7fffea2ae6da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #13 0x7fffea9f388e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x12188e)

0x60400327a4d0 is located 0 bytes inside of 40-byte region [0x60400327a4d0,0x60400327a4f8)
freed by thread T0 here:
    #0 0x7ffff6ef87b8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8)
    #1 0x7fffeb37b055 in QMapDataBase::freeNodeAndRebalance(QMapNodeBase*) (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0x117055)

previously allocated by thread T1039 (Thread (pooled)) here:
    #0 0x7ffff6ef8b50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x7fffeb37b430 in QMapDataBase::createNode(int, int, QMapNodeBase*, bool) (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0x117430)

Thread T1039 (Thread (pooled)) created by T0 here:
    #0 0x7ffff6e51d2f in __interceptor_pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x37d2f)
    #1 0x7fffeb30b2ed in QThread::start(QThread::Priority) (/usr/lib/x86_64-linux-gnu/libQt5Core.so.5+0xa72ed)

SUMMARY: AddressSanitizer: double-free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8) in __interceptor_free
==1199==ABORTING
Comment 2 wolthera 2019-08-04 13:21:21 UTC
This is firmly in the rendering system(and doesn't seem to have to do with tusooaa's vector pointer work), so I guess we should assign Dmitry.
Comment 3 Dmitry Kazakov 2019-08-14 16:16:45 UTC
Git commit a5ef0656ef75ecb477883efa99b6a9c435943df0 by Dmitry Kazakov.
Committed on 14/08/2019 at 16:16.
Pushed by dkazakov into branch 'master'.

Remove update compressor in KoShapeManager

Anyway we always recalculate tree before any access to the shapes
BACKPORT:krita/4.2

M  +1    -5    libs/flake/KoShapeManager.cpp
M  +1    -5    libs/flake/KoShapeManager_p.h

https://invent.kde.org/kde/krita/commit/a5ef0656ef75ecb477883efa99b6a9c435943df0
Comment 4 Dmitry Kazakov 2019-08-14 16:16:45 UTC
Git commit 1e5db24a24c2e3413f5a533e5a6efd92d772d2ae by Dmitry Kazakov.
Committed on 14/08/2019 at 16:16.
Pushed by dkazakov into branch 'master'.

Fix crash when creating a bezier curve

The patch basically makes KoShapeManager thread safe by adding
a simple mutex. The problem is that both,
KoCreatePathTool::Private::endPointAtPosition() and
KisRepaintShapeLayerLayerJob access the shape manager in different
threads concurrently, which obviously causes a crash.
BACKPORT:krita/4.2

M  +22   -0    libs/flake/KoShapeManager.cpp
M  +2    -0    libs/flake/KoShapeManager_p.h
M  +1    -1    libs/ui/tool/kis_tool_shape.cc

https://invent.kde.org/kde/krita/commit/1e5db24a24c2e3413f5a533e5a6efd92d772d2ae
Comment 5 Dmitry Kazakov 2019-08-14 16:41:49 UTC
Git commit aed25510add2f046b4cd96f0c4b4719a4d00fae1 by Dmitry Kazakov.
Committed on 14/08/2019 at 16:22.
Pushed by dkazakov into branch 'krita/4.2'.

Fix crash when creating a bezier curve

The patch basically makes KoShapeManager thread safe by adding
a simple mutex. The problem is that both,
KoCreatePathTool::Private::endPointAtPosition() and
KisRepaintShapeLayerLayerJob access the shape manager in different
threads concurrently, which obviously causes a crash.
BACKPORT:krita/4.2

M  +22   -0    libs/flake/KoShapeManager.cpp
M  +2    -0    libs/flake/KoShapeManager_p.h
M  +1    -1    libs/ui/tool/kis_tool_shape.cc

https://invent.kde.org/kde/krita/commit/aed25510add2f046b4cd96f0c4b4719a4d00fae1
Comment 6 Dmitry Kazakov 2019-08-14 16:41:50 UTC
Git commit 60912d76c0738e3fe58aad14ce2938e27976e88f by Dmitry Kazakov.
Committed on 14/08/2019 at 16:21.
Pushed by dkazakov into branch 'krita/4.2'.

Remove update compressor in KoShapeManager

Anyway we always recalculate tree before any access to the shapes
BACKPORT:krita/4.2

M  +1    -5    libs/flake/KoShapeManager.cpp
M  +1    -5    libs/flake/KoShapeManager_p.h

https://invent.kde.org/kde/krita/commit/60912d76c0738e3fe58aad14ce2938e27976e88f
Comment 7 Dmitry Kazakov 2019-08-14 19:43:24 UTC
The fix makes Krita freeze
Comment 8 Dmitry Kazakov 2019-08-15 13:56:32 UTC
Git commit 1ec268fac79b1e16d20cf84739febd3e7750821d by Dmitry Kazakov.
Committed on 15/08/2019 at 13:55.
Pushed by dkazakov into branch 'master'.

Fix deadlocks in KoShapeManager caused by bezier curve fix

1) Some shapes (e.g. Text Shape) may emit shape-changed signal right
   during painting. It would cause a deadlock.

2) add/removeShape methods should be more careful about locking
   because of their tail-recursion nature.
Related: bug 410909

M  +78   -45   libs/flake/KoShapeManager.cpp
M  +2    -1    libs/flake/KoShapeManager_p.h

https://invent.kde.org/kde/krita/commit/1ec268fac79b1e16d20cf84739febd3e7750821d
Comment 9 Dmitry Kazakov 2019-08-15 15:24:11 UTC
*** Bug 410941 has been marked as a duplicate of this bug. ***
Comment 10 Dmitry Kazakov 2019-08-15 15:25:00 UTC
Git commit c025b251079158f2206c3a1d29b3068062310077 by Dmitry Kazakov.
Committed on 15/08/2019 at 14:40.
Pushed by dkazakov into branch 'krita/4.2'.

Fix crash when creating a bezier curve

The patch basically makes KoShapeManager thread safe by adding
a simple mutex. The problem is that both,
KoCreatePathTool::Private::endPointAtPosition() and
KisRepaintShapeLayerLayerJob access the shape manager in different
threads concurrently, which obviously causes a crash.
BACKPORT:krita/4.2

M  +22   -0    libs/flake/KoShapeManager.cpp
M  +2    -0    libs/flake/KoShapeManager_p.h
M  +1    -1    libs/ui/tool/kis_tool_shape.cc

https://invent.kde.org/kde/krita/commit/c025b251079158f2206c3a1d29b3068062310077
Comment 11 Dmitry Kazakov 2019-08-15 15:25:00 UTC
Git commit c2d3c64b9be35b9ade4658f7b26e0a86f38e9a7c by Dmitry Kazakov.
Committed on 15/08/2019 at 14:41.
Pushed by dkazakov into branch 'krita/4.2'.

Fix deadlocks in KoShapeManager caused by bezier curve fix

1) Some shapes (e.g. Text Shape) may emit shape-changed signal right
   during painting. It would cause a deadlock.

2) add/removeShape methods should be more careful about locking
   because of their tail-recursion nature.
Related: bug 410909

M  +78   -45   libs/flake/KoShapeManager.cpp
M  +2    -1    libs/flake/KoShapeManager_p.h

https://invent.kde.org/kde/krita/commit/c2d3c64b9be35b9ade4658f7b26e0a86f38e9a7c