| Summary: | Crash in uninitialized transform tool | ||
|---|---|---|---|
| Product: | [Applications] krita | Reporter: | Tiar <tamtamy.tymona> |
| Component: | Tools/Transform | Assignee: | Tiar <tamtamy.tymona> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | dan.mclaughlin, defresne.thierry, halla, kiryasolod, leviatan1, pedroreis.ad, pineapplewitness, piotr.panasiuk, rubstov, sebastianlabi |
| Priority: | NOR | Keywords: | regression, release_blocker |
| Version: | git master (please specify the git hash!) | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/kde/krita/commit/47423c9abab188a53c4003a09ce6bdf0824baef9 | Version Fixed In: | |
| Sentry Crash Report: | |||
| Attachments: |
gdb backtrace - thread apply all bt
gdb backtrace - thread apply all bt full gdb backtrace (2) - thread apply all bt gdb backtrace (2) - thread apply all bt full gdb backtrace (3) - thread apply all bt gdb backtrace (3) - thread apply all bt full Backtrace trying to reproduce, identical to the one from bug 415625 |
||
|
Description
Tiar
2019-11-30 00:21:22 UTC
Created attachment 124196 [details]
gdb backtrace - thread apply all bt full
Created attachment 124199 [details]
gdb backtrace (2) - thread apply all bt
I was painting inside gdb and noticed this backtrace is slightly different (no cut/copy, only TileManager and TransformTool threads)
Created attachment 124200 [details]
gdb backtrace (2) - thread apply all bt full
*** Bug 414638 has been marked as a duplicate of this bug. *** *** Bug 414645 has been marked as a duplicate of this bug. *** Created attachment 124221 [details]
gdb backtrace (3) - thread apply all bt
I've found a good way to reproduce it. It isn't the nicest backtrace like the second one, but it's reproducible at least.
1. Create a new document.
2. Create a new layer.
3. Use Ellipse Tool (I created a perfect circle).
4. Press ctrl+T to get to the Transform Tool.
5. Select Shape Selection Tool (arrow tool).
Note: if you first transform the circle and then select the other tool, it won't happen.
Created attachment 124222 [details]
gdb backtrace (3) - thread apply all bt full
The crash was introduced in commit f336923b87. The cause seems to be calling endStroke() in kis_tool_transform twice, and so fast that the second one doesn't know the first one already started. Assigning to Dmitry since it's a regression and caused by his changes. Setting to "release_blocker" because we've got two reports on that besides mine in just a few days and there is just no way to work with transform tool now, so I believe it's quite important. I hope it's not an overstepping of my powers or anything ;) I believe I'm seeing this too. Start with an image, select transform tool, then paste another image into the first and you get a crash. Command line (Linux) output is krita: /usr/include/boost/optional/optional.hpp:1191: boost::optional<T>::reference_type boost::optional<T>::get() [with T = ToolTransformArgs; boost::optional<T>::reference_type = ToolTransformArgs&]: Assertion `this->is_initialized()' failed. /home/dan/Applications/startKrita: line 1: 12553 Aborted OK, I'm getting crashes all over the place with the Transform tool, same error output as reported above. Just selecting a different layer, while the Transform tool is enabled, causes a crash *** Bug 414797 has been marked as a duplicate of this bug. *** Git commit 2dc2ed5f5a6be2b81aab5001de8389e3f3cbdcac by Dmitry Kazakov. Committed on 04/12/2019 at 17:41. Pushed by dkazakov into branch 'master'. Fix a crash when cancelling Transform Tool action The crash happens only on systems that have asserts enabled, that is, don't have NDEBUG defined. Binding uninitialized '*m_savedTransformArgs' to a cont-reference generated valid c++ code (a reference initialized with nullptr), and given that finishStrokeImpl() didn't try to use/dereference this reference, the code worked fine and didn't crash on systems without asserts. But on systems with asserts enabled (non NDEBUG), boost::optional triggered a sanity check assert for dereferencing nullptr and crashed the application. The patch removes entire code for resetting m_savedTransformArgs in cancelStrokeCallback(). It was actually an artifact of some initial refactoring. Thanks Fredrik Hansson for pointing out the problem! https://invent.kde.org/kde/krita/merge_requests/197 M +1 -8 plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp https://invent.kde.org/kde/krita/commit/2dc2ed5f5a6be2b81aab5001de8389e3f3cbdcac *** Bug 415090 has been marked as a duplicate of this bug. *** *** Bug 415388 has been marked as a duplicate of this bug. *** *** Bug 415675 has been marked as a duplicate of this bug. *** *** Bug 415654 has been marked as a duplicate of this bug. *** I'm still getting this all the time. Simple create an image, select the move tool, then the transform tool will assert: krita: /usr/include/boost/optional/optional.hpp:1107: boost::optional<T>::reference_type boost::optional<T>::get() [with T = ToolTransformArgs; boost::optional<T>::reference_type = ToolTransformArgs&]: Assertion `this->is_initialized()' failed. KCrash: crashing... crashRecursionCounter = 2 KCrash: Application Name = krita path = /home/boud/dev/i-4.2/bin pid = 14253 KCrash: Arguments: /home/boud/dev/i-4.2/bin/krita Created attachment 124975 [details] Backtrace trying to reproduce, identical to the one from bug 415625 I'm on 3f0ff1a86c + a few irrelevant commits. When I try to reproduce according to boud's steps: - (I first select Move Tool) - it doesn't assert after changing from Move Tool to Transform Tool - it does crash when changing from Transform Tool to Freehand Brush Tool... but the crash log seem to be identical with what I would get in bug 415625: So @boud if you cannot reproduce it now after fixing bug 415625, that might be a reason and this bug should be closed as well? Thread 31 "Thread (pooled)" received signal SIGABRT, Aborted. [Switching to Thread 0x7fff97fff700 (LWP 27201)] __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory. (gdb) thread apply all bt Thread 31 (Thread 0x7fff97fff700 (LWP 27201)): #0 0x00007ffff4262077 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007ffff4243535 in __GI_abort () at abort.c:79 #2 0x00007ffff46318d7 in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #3 0x00007ffff4630d59 in qt_assert_x(char const*, char const*, char const*, int) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #4 0x00007ffff5a15528 in KisStroke::addJob(KisStrokeJobData*) (this=<optimized out>, data=<optimized out>) at /home/tymon/devsec/krita/libs/image/kis_stroke.cpp:84 #5 0x00007ffff5c828e2 in KisStrokesQueue::addJob(QWeakPointer<KisStroke>, KisStrokeJobData*) (this=<optimized out>, id=..., data=0x7fffb80017c0) at /home/tymon/devsec/krita/libs/image/kis_strokes_queue.cpp:339 #6 0x00007ffff5c8fbc0 in KisUpdateScheduler::addJob(QWeakPointer<KisStroke>, KisStrokeJobData*) (this=0x5555739a4c80, id=..., data=<optimized out>) at /usr/include/c++/8/bits/atomic_base.h:295 #7 0x00007ffff5cb8e2f in non-virtual thunk to KisImage::addJob(QWeakPointer<KisStroke>, KisStrokeJobData*) () at /usr/include/c++/8/bits/atomic_base.h:295 #8 0x00007ffff5b75bb2 in KisSavedMacroCommand::addCommands(QWeakPointer<KisStroke>, bool) (this=0x555570ba9df0, id=..., undo=<optimized out>) at /usr/include/c++/8/bits/atomic_base.h:295 #9 0x00007ffff5b74802 in KisSavedMacroCommand::performCancel(QWeakPointer<KisStroke>, bool) (this=<optimized out>, id=..., strokeUndo=<optimized out>) at /usr/include/c++/8/bits/atomic_base.h:295 #10 0x00007ffff5c7de85 in KisStrokeStrategyUndoCommandBased::cancelStrokeCallback() (this=0x55555bb90b60) at /usr/include/c++/8/bits/atomic_base.h:295 #11 0x00007fffd495a2a5 in TransformStrokeStrategy::<lambda()>::operator() (__closure=0x7fffb0004c18, __closure=0x7fffb0004c18) at /home/tymon/devsec/krita/plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp:686 #12 0x00007fffd495a2a5 in std::_Function_handler<void (), TransformStrokeStrategy::finishStrokeImpl(bool, ToolTransformArgs const&)::{lambda()#1}>::_M_invoke(std::_Any_data const&) (__functor=...) at /usr/include/c++/8/bits/std_function.h:297 #13 0x00007ffff5c7e05b in KisStrokeStrategyUndoCommandBased::doStrokeCallback(KisStrokeJobData*) (this=0x55555bb90b60, data=0x7fffb0004bf0) at /home/tymon/devsec/krita/libs/image/kis_stroke_strategy_undo_command_based.cpp:131 #14 0x00007fffd495e59c in TransformStrokeStrategy::doStrokeCallback(KisStrokeJobData*) (this=0x55555bb90b50, data=0x7fffb0004bf0) at /home/tymon/devsec/krita/plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp:350 #15 0x00007ffff5e988cc in non-virtual thunk to KisUpdateJobItem::run() () at /usr/include/c++/8/bits/atomic_base.h:295 #16 0x00007ffff466af71 in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #17 0x00007ffff4672c87 in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #18 0x00007ffff206e164 in start_thread (arg=<optimized out>) at pthread_create.c:486 #19 0x00007ffff433bdef in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Git commit 47423c9abab188a53c4003a09ce6bdf0824baef9 by Boudewijn Rempt, on behalf of Dmitry Kazakov. Committed on 08/01/2020 at 14:13. Pushed by rempt into branch 'krita/4.2'. Fix a crash when cancelling Transform Tool action The crash happens only on systems that have asserts enabled, that is, don't have NDEBUG defined. Binding uninitialized '*m_savedTransformArgs' to a cont-reference generated valid c++ code (a reference initialized with nullptr), and given that finishStrokeImpl() didn't try to use/dereference this reference, the code worked fine and didn't crash on systems without asserts. But on systems with asserts enabled (non NDEBUG), boost::optional triggered a sanity check assert for dereferencing nullptr and crashed the application. The patch removes entire code for resetting m_savedTransformArgs in cancelStrokeCallback(). It was actually an artifact of some initial refactoring. Thanks Fredrik Hansson for pointing out the problem! https://invent.kde.org/kde/krita/merge_requests/197 (cherry picked from commit 2dc2ed5f5a6be2b81aab5001de8389e3f3cbdcac) M +1 -8 plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp https://invent.kde.org/kde/krita/commit/47423c9abab188a53c4003a09ce6bdf0824baef9 Um... Now I can make the content of a layer completely disappear by first painting, then clicking the transform tool, then the move tool: SAFE ASSERT (krita): "!m_strokeEnded || m_isCancelled" in file /home/boud/dev/krita/libs/image/kis_stroke.cpp, line 84 SAFE ASSERT (krita): "!m_strokeEnded || m_isCancelled" in file /home/boud/dev/krita/libs/image/kis_stroke.cpp, line 84 SAFE ASSERT (krita): "!m_strokeEnded || m_isCancelled" in file /home/boud/dev/krita/libs/image/kis_stroke.cpp, line 84 There's still something extremely fishy going on, and my commit making a safe assert on this line very probably is wrong. Note: there are also bug 402770 and bug 408057 that might be responsible for that. Git commit 8ef04427c4b8e63265ab4bb3456659cf4b0d39a4 by Agata Cacko. Committed on 14/01/2020 at 22:14. Pushed by tymond into branch 'master'. Initialize transform tool args when ending stroke Before this commit, transform tool would get an assert when switching to another tool without any changes. This commit removes trying to cancel the stroke in the finishStrokeCallback() call and initializes m_savedTransformArgs so the finishing can happen as always. WARNING: it probably isn't the "proper" solution for this issue. It does work correctly, but it adds an unnecessary undo step in the situation when cancelling should be used instead of finishing the stroke (for example when the user switches to transform tool and then immediately to another tool without making any changes). Related: bug 415625 M +25 -0 plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp https://invent.kde.org/kde/krita/commit/8ef04427c4b8e63265ab4bb3456659cf4b0d39a4 *** Bug 416468 has been marked as a duplicate of this bug. *** Git commit 4ec9beafc1d40205b82aa50dc6f981cc10891d0f by Boudewijn Rempt, on behalf of Agata Cacko. Committed on 28/01/2020 at 09:09. Pushed by rempt into branch 'krita/4.2'. Initialize transform tool args when ending stroke Before this commit, transform tool would get an assert when switching to another tool without any changes. This commit removes trying to cancel the stroke in the finishStrokeCallback() call and initializes m_savedTransformArgs so the finishing can happen as always. WARNING: it probably isn't the "proper" solution for this issue. It does work correctly, but it adds an unnecessary undo step in the situation when cancelling should be used instead of finishing the stroke (for example when the user switches to transform tool and then immediately to another tool without making any changes). Related: bug 415625 (cherry picked from commit 8ef04427c4b8e63265ab4bb3456659cf4b0d39a4) M +25 -0 plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp https://invent.kde.org/kde/krita/commit/4ec9beafc1d40205b82aa50dc6f981cc10891d0f |