Differential D3051

[applets/notification] Add nullptr check
AbandonedPublic
Actions

Authored by graesslin on Oct 14 2016, 6:17 AM.

Details

Reviewers
sebas
broulik
Group Reviewers
Plasma
Summary

Hit a crash there, this change prevents it:

Thread 1 "plasmashell" received signal SIGSEGV, Segmentation fault.
QObject::setProperty (this=0x0, name=0x7fff2dd11a68
"initialPositionSet", value=...) at kernel/qobject.cpp:3849
3849 const QMetaObject* meta = metaObject();
(gdb) bt
#0 QObject::setProperty (this=0x0, name=0x7fff2dd11a68
"initialPositionSet", value=...) at kernel/qobject.cpp:3849
#1 0x00007fff2dd09d0b in
NotificationsHelper::addNotificationPopup(QObject*) ()

from

/opt/kf5/lib/x86_64-linux-gnu/qml/org/kde/plasma/private/notifications/libnotificationshelperplugin.so
#2 0x00007fff2dd1106d in
NotificationsHelper::qt_static_metacall(QObject*, QMetaObject::Call,
int, void**) ()

from

/opt/kf5/lib/x86_64-linux-gnu/qml/org/kde/plasma/private/notifications/libnotificationshelperplugin.so
#3 0x00007fff2dd112a3 in
NotificationsHelper::qt_metacall(QMetaObject::Call, int, void**) ()

from

/opt/kf5/lib/x86_64-linux-gnu/qml/org/kde/plasma/private/notifications/libnotificationshelperplugin.so
#4 0x00007ffff4eddc99 in QQmlObjectOrGadget::metacall
(this=this@entry=0x7fffffff3bd0,
type=type@entry=QMetaObject::InvokeMetaMethod, index=<optimized out>,
index@entry=11,

argv=<optimized out>) at qml/qqmlpropertycache.cpp:1547

#5 0x00007ffff4e70028 in CallMethod (object=..., index=<optimized out>,
returnType=returnType@entry=43, argCount=<optimized out>,
argTypes=<optimized out>,

engine=engine@entry=0x849cf0, callArgs=<optimized out>) at

jsruntime/qv4qobjectwrapper.cpp:1137
#6 0x00007ffff4e71478 in CallPrecise (object=..., data=...,
engine=engine@entry=0x849cf0, callArgs=callArgs@entry=0x7fffdc230558) at
jsruntime/qv4qobjectwrapper.cpp:1388
#7 0x00007ffff4e71ced in QV4::QObjectMethod::callInternal
(this=<optimized out>, callData=<optimized out>) at
jsruntime/qv4qobjectwrapper.cpp:1871
#8 0x00007ffff4e86d21 in QV4::Object::call (d=0x7fffdc230558,
this=<optimized out>) at jsruntime/qv4object_p.h:332
#9 QV4::Runtime::callProperty (engine=0x849cf0, nameIndex=<optimized
out>, callData=0x7fffdc230558) at jsruntime/qv4runtime.cpp:1030
#10 0x00007fff2dce8152 in ?? ()
#11 0x3a3a646e616c7961 in ?? ()
#12 0x3a3a746e65696c43 in ?? ()
#13 0x0000000000000000 in ?? ()

Diff Detail

Repository
R120 Plasma Workspace
Branch
notification-add-popup-null
Lint
No Linters Available
Unit
No Unit Test Coverage
graesslin updated this revision to Diff 7383.Oct 14 2016, 6:17 AM
graesslin retitled this revision from to [applets/notification] Add nullptr check.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added reviewers: Plasma, broulik.
Restricted Application added a project: Plasma. · View Herald TranscriptOct 14 2016, 6:17 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik edited edge metadata.Oct 14 2016, 7:07 AM

While in principle the patch is fine, the only way this can be null is when NotificationPopup.qml (which is a Plasma Dialog) failed to load, so I think we're masking a deeper problem here.

var popup = notificationPopupComponent.createObject();
notificationPositioner.addNotificationPopup(popup);
sebas added a subscriber: sebas.Oct 16 2016, 2:58 PM

So, warn, but don't crash?

graesslin added a comment.Oct 16 2016, 3:35 PM
In D3051#56876, @sebas wrote:

So, warn, but don't crash?

It might be that this was a local qt self-destruction event. I recompiled Qt on Thursday and afterwards everything was broken, especially QtQuick. It might be that this was the reason for the null popup here. I'll try another recompile next week. In case thinks work again and it's not needed, I'll discard the review.

mart added a subscriber: mart.Nov 7 2016, 2:03 PM

any news on this? i don't see issues with the patch, even if obviously kindof a workaround

sebas accepted this revision.Dec 5 2016, 1:47 PM
sebas added a reviewer: sebas.

So, let's merge this? It's small enough...

This revision is now accepted and ready to land.Dec 5 2016, 1:47 PM
sebas added a comment.Jun 14 2017, 11:41 AM

Ping? Please submit or abandon...

graesslin abandoned this revision.Jun 15 2017, 10:46 AM

Revision Contents
Changeset List

Diff 7383

View Options

applets/notifications/plugin/notificationshelper.cpp

Loading...