Differential D20659

Copy container in Component::cleanUp before interating
ClosedPublic
Actions

Authored by fvogt on Apr 18 2019, 12:32 PM.

Details

Reviewers
davidedmundson
Group Reviewers
Frameworks
Commits
R268:78a711361db3: Copy container in Component::cleanUp before interating
Summary

Crash was reported:

Thread 1 (Thread 0x7fdc95c68800 (LWP 6402)):
[KCrash Handler]
#6 QHashData::nextNode (node=node@entry=0x562f53ffbd10) at tools/qhash.cpp:598
#7 0x00007fdc95a1fbab in QHash<QString, GlobalShortcut*>::const_iterator::operator++ (this=<synthetic pointer>) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qhash.h:395
#8 KdeDGlobalAccel::Component::cleanUp (this=0x562f53ffb040) at ./src/runtime/component.cpp:163

Apparently the container is modified while iterating over it. That does not work with the range-for as it does not detach, as opposed to Q_FOREACH.

Test Plan

@lbeltrame saw valgrind errors before applying this, but those disappeared with this patch.

Diff Detail

Repository
R268 KGlobalAccel
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
fvogt created this revision.Apr 18 2019, 12:32 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 18 2019, 12:32 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
fvogt requested review of this revision.Apr 18 2019, 12:32 PM
fvogt updated this revision to Diff 56517.Apr 18 2019, 12:34 PM

Use auto (which might actually make it build)

lbeltrame added a subscriber: lbeltrame.Apr 18 2019, 12:53 PM

I confirm there are no more issues in valgrind after adding this patch.

fvogt edited the summary of this revision. (Show Details)Apr 18 2019, 12:57 PM
fvogt edited the test plan for this revision. (Show Details)
fvogt retitled this revision from Detach container in Component::cleanUp before interating to Copy container in Component::cleanUp before interating.
davidedmundson accepted this revision.Apr 18 2019, 1:14 PM
This revision is now accepted and ready to land.Apr 18 2019, 1:14 PM
Closed by commit R268:78a711361db3: Copy container in Component::cleanUp before interating (authored by fvogt). · Explain WhyApr 18 2019, 1:16 PM
This revision was automatically updated to reflect the committed changes.
fvogt added a comment.Apr 18 2019, 6:06 PM
In D20659#452642, @ngraham wrote:

Did this fix https://bugs.kde.org/show_bug.cgi?id=406642?

No, that particular crash (bug 406426) is already fixed. I marked it as dup.

Revision Contents
Changeset List

PathPackages
Msrc/runtime/component.cpp (3 lines)

Diff 56524

View Options

src/runtime/component.cpp

Loading...