Krita: patch for a crash during document close
ClosedPublic
Actions

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.

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

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.

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.

fazek mentioned this in D1195: Tile management with shared pointers instead of reference counting.Mar 23 2016, 4:37 PM

fazek added a reviewer: dkazakov.Mar 23 2016, 6:24 PM

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

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 mentioned this in R37:8fef07fc4906: D1199 patch for a crash during document close.Mar 25 2016, 9:05 AM

fazek closed this revision.Mar 25 2016, 9:05 AM