Differential D28075

[kstyle] Properly unregister widgets in ShadowHelper
ClosedPublic
Actions

Authored by zzag on Mar 16 2020, 1:41 PM.

Details

Reviewers
cblack
Group Reviewers
Plasma
Commits
R31:49fdd73e1629: [kstyle] Properly unregister widgets in ShadowHelper
Summary

After a widget has been unregistered, a WinIdChange event may be sent.
If that happens, ShadowHelper is going to try to install a shadow for
the corresponding widget. Obviously, this is wrong.

Diff Detail

Repository
R31 Breeze
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
zzag created this revision.Mar 16 2020, 1:41 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 16 2020, 1:41 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
zzag requested review of this revision.Mar 16 2020, 1:41 PM
cblack accepted this revision.Mar 16 2020, 1:49 PM
This revision is now accepted and ready to land.Mar 16 2020, 1:49 PM
Closed by commit R31:49fdd73e1629: [kstyle] Properly unregister widgets in ShadowHelper (authored by zzag). · Explain WhyMar 18 2020, 10:08 AM
This revision was automatically updated to reflect the committed changes.
ahmadsamir added a subscriber: ahmadsamir.Mar 18 2020, 11:57 AM

FWIW, I have the kdatetimeedittest unit test from KWidgetAddons fail with a SIGSEGV even after building breeze with this diff:

Thread 1 "kdatetimeeditte" received signal SIGSEGV, Segmentation fault.
0x00007ffff0c6c770 in qDeleteAll<QMap<QWidget*, KWindowShadow*>::const_iterator> (begin=..., end=...)
    at /usr/include/qt5/QtCore/qalgorithms.h:320
320             delete *begin;
(gdb) bt
#0  0x00007ffff0c6c770 in qDeleteAll<QMap<QWidget*, KWindowShadow*>::const_iterator> (begin=..., end=...)
    at /usr/include/qt5/QtCore/qalgorithms.h:320
#1  0x00007ffff0c6bdb8 in qDeleteAll<QMap<QWidget*, KWindowShadow*> > (c=...) at /usr/include/qt5/QtCore/qalgorithms.h:328
#2  0x00007ffff0c69319 in Breeze::ShadowHelper::~ShadowHelper (this=0x547ca0, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezeshadowhelper.cpp:106
#3  0x00007ffff0c69380 in Breeze::ShadowHelper::~ShadowHelper (this=0x547ca0, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezeshadowhelper.cpp:107
#4  0x00007ffff0c701a2 in Breeze::Style::~Style (this=0x547a40, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezestyle.cpp:200
#5  0x00007ffff0c70206 in Breeze::Style::~Style (this=0x547a40, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezestyle.cpp:202
#6  0x00007ffff77ff5e8 in QApplication::~QApplication (this=0x7fffffffdd00, __in_chrg=<optimized out>) at kernel/qapplication.cpp:812
#7  0x0000000000408f65 in main (argc=1, argv=0x7fffffffde58) at /home/ahmad/rpmbuild/dev/kwidgetsaddons/autotests/kdatetimeedittest.cpp:31
zzag added a comment.Mar 18 2020, 11:58 AM
In D28075#629808, @ahmadsamir wrote:

FWIW, I have the kdatetimeedittest unit test from KWidgetAddons fail with a SIGSEGV even after building breeze with this diff:

Thread 1 "kdatetimeeditte" received signal SIGSEGV, Segmentation fault.
0x00007ffff0c6c770 in qDeleteAll<QMap<QWidget*, KWindowShadow*>::const_iterator> (begin=..., end=...)
    at /usr/include/qt5/QtCore/qalgorithms.h:320
320             delete *begin;
(gdb) bt
#0  0x00007ffff0c6c770 in qDeleteAll<QMap<QWidget*, KWindowShadow*>::const_iterator> (begin=..., end=...)
    at /usr/include/qt5/QtCore/qalgorithms.h:320
#1  0x00007ffff0c6bdb8 in qDeleteAll<QMap<QWidget*, KWindowShadow*> > (c=...) at /usr/include/qt5/QtCore/qalgorithms.h:328
#2  0x00007ffff0c69319 in Breeze::ShadowHelper::~ShadowHelper (this=0x547ca0, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezeshadowhelper.cpp:106
#3  0x00007ffff0c69380 in Breeze::ShadowHelper::~ShadowHelper (this=0x547ca0, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezeshadowhelper.cpp:107
#4  0x00007ffff0c701a2 in Breeze::Style::~Style (this=0x547a40, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezestyle.cpp:200
#5  0x00007ffff0c70206 in Breeze::Style::~Style (this=0x547a40, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezestyle.cpp:202
#6  0x00007ffff77ff5e8 in QApplication::~QApplication (this=0x7fffffffdd00, __in_chrg=<optimized out>) at kernel/qapplication.cpp:812
#7  0x0000000000408f65 in main (argc=1, argv=0x7fffffffde58) at /home/ahmad/rpmbuild/dev/kwidgetsaddons/autotests/kdatetimeedittest.cpp:31

Urgh, thanks. I'm gonna have a look.

zzag added a comment.Mar 18 2020, 12:02 PM
In D28075#629808, @ahmadsamir wrote:

FWIW, I have the kdatetimeedittest unit test from KWidgetAddons fail with a SIGSEGV even after building breeze with this diff:

Thread 1 "kdatetimeeditte" received signal SIGSEGV, Segmentation fault.
0x00007ffff0c6c770 in qDeleteAll<QMap<QWidget*, KWindowShadow*>::const_iterator> (begin=..., end=...)
    at /usr/include/qt5/QtCore/qalgorithms.h:320
320             delete *begin;
(gdb) bt
#0  0x00007ffff0c6c770 in qDeleteAll<QMap<QWidget*, KWindowShadow*>::const_iterator> (begin=..., end=...)
    at /usr/include/qt5/QtCore/qalgorithms.h:320
#1  0x00007ffff0c6bdb8 in qDeleteAll<QMap<QWidget*, KWindowShadow*> > (c=...) at /usr/include/qt5/QtCore/qalgorithms.h:328
#2  0x00007ffff0c69319 in Breeze::ShadowHelper::~ShadowHelper (this=0x547ca0, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezeshadowhelper.cpp:106
#3  0x00007ffff0c69380 in Breeze::ShadowHelper::~ShadowHelper (this=0x547ca0, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezeshadowhelper.cpp:107
#4  0x00007ffff0c701a2 in Breeze::Style::~Style (this=0x547a40, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezestyle.cpp:200
#5  0x00007ffff0c70206 in Breeze::Style::~Style (this=0x547a40, __in_chrg=<optimized out>)
    at /home/ahmad/rpmbuild/dev/breeze/kstyle/breezestyle.cpp:202
#6  0x00007ffff77ff5e8 in QApplication::~QApplication (this=0x7fffffffdd00, __in_chrg=<optimized out>) at kernel/qapplication.cpp:812
#7  0x0000000000408f65 in main (argc=1, argv=0x7fffffffde58) at /home/ahmad/rpmbuild/dev/kwidgetsaddons/autotests/kdatetimeedittest.cpp:31

Hmm,

❯ dbus-run-session bin/kdatetimeedittest
********* Start testing of KDateTimeEditTest *********
Config: Using QtTest library 5.15.0, Qt 5.15.0 (x86_64-little_endian-lp64 shared (dynamic) debug build; by GCC 9.2.1 20200130)
PASS   : KDateTimeEditTest::initTestCase()
PASS   : KDateTimeEditTest::_q_showIfNotHidden()
PASS   : KDateTimeEditTest::testDefaults()
PASS   : KDateTimeEditTest::testValidNull()
PASS   : KDateTimeEditTest::testDateTimeRange()
PASS   : KDateTimeEditTest::testOptions()
PASS   : KDateTimeEditTest::testDateDisplayFormat()
PASS   : KDateTimeEditTest::testDateList()
PASS   : KDateTimeEditTest::testTimeListInterval()
PASS   : KDateTimeEditTest::testTimeList()
PASS   : KDateTimeEditTest::testTimeDisplayFormat()
PASS   : KDateTimeEditTest::testCalendarSystem()
PASS   : KDateTimeEditTest::testTimeSpec()
PASS   : KDateTimeEditTest::testDateMenu()
PASS   : KDateTimeEditTest::cleanupTestCase()
Totals: 15 passed, 0 failed, 0 skipped, 0 blacklisted, 817ms
********* Finished testing of KDateTimeEditTest *********
ahmadsamir added a comment.Mar 18 2020, 12:15 PM

Could be something on my system.... no idea what though. Sorry for the noise.

Anyway, the important thing is that the test passes on the CI system.

zzag added a comment.Mar 18 2020, 12:44 PM
In D28075#629829, @ahmadsamir wrote:

Anyway, the important thing is that the test passes on the CI system.

Okay, that's good! We still have that qDeleteAll in ~ShadowHelper(), not sure what we have to do about it.

Revision Contents
Changeset List

PathPackages
Mkstyle/breezeshadowhelper.cpp (11 lines)

Diff 77893

View Options

kstyle/breezeshadowhelper.cpp

Loading...