Krita: patch for a crash during document close
ClosedPublic

Authored by fazek on Mar 22 2016, 6:25 PM.

Details

Reviewers
dkazakov
rempt
Summary

This crash happened to me several times. After the image deleted, sometimes a last slotInfoConverted() call arrives to the animation cache populator. After that the program crashes in this addConvertedFrameData() function because it tries to use a null pointer.

I'm not sure if this is a correct solution, or maybe some additional steps needed to avoid a memory leak.

This is the stack dump:

#0 KisNode::getKeyframeChannel (this=this@entry=0x0, id=...) at /home/fazekas/Asztal/krita-dev/krita/libs/image/kis_node.cpp:264
#1 0x00007f77d5daf3d8 in KisTimeRange::calculateTimeRangeRecursive (node=0x0, time=time@entry=8, range=...,

exclusive=exclusive@entry=true) at /home/fazekas/Asztal/krita-dev/krita/libs/image/kis_time_range.cpp:43

#2 0x00007f77d97255f9 in KisAnimationFrameCache::addConvertedFrameData (this=0xa1b4210, info=..., time=8)

at /home/fazekas/Asztal/krita-dev/krita/libs/ui/kis_animation_frame_cache.cpp:234

#3 0x00007f77d9726c9c in infoConverted (this=0x83890e0)

at /home/fazekas/Asztal/krita-dev/krita/libs/ui/kis_animation_cache_populator.cpp:117

#4 KisAnimationCachePopulator::slotInfoConverted (this=<optimized out>)

at /home/fazekas/Asztal/krita-dev/krita/libs/ui/kis_animation_cache_populator.cpp:359

#5 0x00007f77d7f6eed6 in QMetaObject::activate(QObject*, int, int, void**) () from /opt/qt54/lib/libQt5Core.so.5
#6 0x00007f77d7d10773 in QFutureWatcherBase::event(QEvent*) () from /opt/qt54/lib/libQt5Core.so.5
...

Test Plan

You cannot force this to happen, sometimes Krita just crashes when you close a document or exit the program.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
fazek updated this revision to Diff 2906.Mar 22 2016, 6:25 PM
fazek retitled this revision from to Krita: patch for a crash during document close.
fazek updated this object.
fazek edited the test plan for this revision. (Show Details)
fazek added a reviewer: rempt.
fazek set the repository for this revision to R37 Krita.
fazek updated this revision to Diff 2909.Mar 22 2016, 6:46 PM

Update: It's strange but sometimes only the root pointer is null... I really hope this is not about threading again...

fazek added a comment.Mar 23 2016, 7:06 AM

I see now when this happens easily. If you modify the animation, after some idle time the program starts building the cache. If you close the document during this, it has a high probability to crash.

fazek updated this revision to Diff 2915.Mar 23 2016, 8:16 AM

So I figured out what is the problem here. The animation interface is part of the private structure of KisImage. But when destroying the image, first its root layer destroyed, and only after that comes the private structure with the animation interface. But if the animation interface has a work in progress, it's possibly wants to use the root layer after it removed. It seems the animation interface is not what waitForDone() waiting for. I decided to create the animation interface after the rootlayer created, and to delete it before the rootlayer deleted. I think this is the clear solution.

rempt edited edge metadata.Mar 24 2016, 6:39 AM

Either Dmitry or Tyyppi should take a look too, but to me it looks correct.

dkazakov accepted this revision.Mar 25 2016, 7:24 AM
dkazakov edited edge metadata.

Hi, @fazek!

Thank you for a nice catch! Please push :)

This revision is now accepted and ready to land.Mar 25 2016, 7:24 AM
fazek closed this revision.Mar 25 2016, 9:05 AM