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
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10991
Build 11009: arc lint + arc unit
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 56517

View Options

src/runtime/component.cpp

Loading...