Differential D14263

Don't attempt to clean up when the process is about to exit
ClosedPublic
Actions

Authored by vkrause on Jul 21 2018, 11:48 AM.

Details

Reviewers
dvratil
Commits
R165:bab79bac9706: Don't attempt to clean up when the process is about to exit
Summary

Fixes a heap-use-after-free ASAN error when the ETMCalendar singleton is
destroyed on shutdown.

14556==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000139910 at pc 0x7fe6291391e3 bp 0x7fff0d420ec0 sp 0x7fff0d420eb8

READ of size 8 at 0x603000139910 thread T0
#0 0x7fe6291391e2 in Akonadi::ChangeNotificationDependenciesFactory::destroyNotificationConnection(Akonadi::Session*, Akonadi::Connection*) /home/jenkins/workspace/Applications akonadi kf5-qt5 SUSEQt5.9/src/core/changenotificationdependenciesfactory.cpp:51
#1 0x7fe629301fcf in Akonadi::MonitorPrivate::disconnectFromNotificationManager() /home/jenkins/workspace/Applications akonadi kf5-qt5 SUSEQt5.9/src/core/monitor_p.cpp:120
#2 0x7fe6293008f2 in Akonadi::MonitorPrivate::~MonitorPrivate() /home/jenkins/workspace/Applications akonadi kf5-qt5 SUSEQt5.9/src/core/monitor_p.cpp:66
#3 0x7fe629300ef9 in Akonadi::MonitorPrivate::~MonitorPrivate() /home/jenkins/workspace/Applications akonadi kf5-qt5 SUSEQt5.9/src/core/monitor_p.cpp:71
#4 0x7fe6292d5c91 in Akonadi::Monitor::~Monitor() /home/jenkins/workspace/Applications akonadi kf5-qt5 SUSEQt5.9/src/core/monitor.cpp:61
#5 0x7fe6292d5cef in Akonadi::Monitor::~Monitor() /home/jenkins/workspace/Applications akonadi kf5-qt5 SUSEQt5.9/src/core/monitor.cpp:62
#6 0x7fe61eebe29a in QObjectPrivate::deleteChildren() (/usr/lib64/libQt5Core.so.5+0x29b29a)
#7 0x7fe61eec6de3 in QObject::~QObject() (/usr/lib64/libQt5Core.so.5+0x2a3de3)
#8 0x7fe5dd5b5a33 in KCalCore::Calendar::~Calendar() /home/jenkins/workspace/Dependency Build Applications kf5-qt5 SUSEQt5.9/kcalcore/src/calendar.cpp:154
#9 0x7fe5dd752127 in KCalCore::MemoryCalendar::~MemoryCalendar() /home/jenkins/workspace/Dependency Build Applications kf5-qt5 SUSEQt5.9/kcalcore/src/memorycalendar.cpp:144
#10 0x7fe5de22bf7c in Akonadi::CalendarBase::~CalendarBase() /home/jenkins/workspace/Dependency Build Applications kf5-qt5 SUSEQt5.9/akonadi-calendar/src/calendarbase.cpp:427
#11 0x7fe5de28d1cc in Akonadi::ETMCalendar::~ETMCalendar() /home/jenkins/workspace/Dependency Build Applications kf5-qt5 SUSEQt5.9/akonadi-calendar/src/etmcalendar.cpp:505
#12 0x7fe5de28d21d in Akonadi::ETMCalendar::~ETMCalendar() /home/jenkins/workspace/Dependency Build Applications kf5-qt5 SUSEQt5.9/akonadi-calendar/src/etmcalendar.cpp:507
#13 0x7fe5dfb473b7 in QtSharedPointer::CustomDeleter<Akonadi::ETMCalendar, QtSharedPointer::NormalDeleter>::execute() (/home/jenkins/install-prefix/lib64/libKF5CalendarSupport.so.5+0x2b93b7)
#14 0x7fe5dfb46bdb in QtSharedPointer::ExternalRefCountWithCustomDeleter<Akonadi::ETMCalendar, QtSharedPointer::NormalDeleter>::deleter(QtSharedPointer::ExternalRefCountData*) /usr/include/qt5/QtCore/qsharedpointer_impl.h:213
#15 0x7fe5dfb2c9f7 in QtSharedPointer::ExternalRefCountData::destroy() /usr/include/qt5/QtCore/qsharedpointer_impl.h:157
#16 0x7fe5dfb2f963 in QSharedPointer<Akonadi::ETMCalendar>::deref(QtSharedPointer::ExternalRefCountData*) /usr/include/qt5/QtCore/qsharedpointer_impl.h:461
#17 0x7fe5dfb2f498 in QSharedPointer<Akonadi::ETMCalendar>::deref() /usr/include/qt5/QtCore/qsharedpointer_impl.h:456
#18 0x7fe5dfb2d9dd in QSharedPointer<Akonadi::ETMCalendar>::~QSharedPointer() /usr/include/qt5/QtCore/qsharedpointer_impl.h:313
#19 0x7fe61df697cb in __run_exit_handlers (/lib64/libc.so.6+0x3a7cb)

Diff Detail

Repository
R165 Akonadi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
vkrause created this revision.Jul 21 2018, 11:48 AM
Restricted Application added a project: KDE PIM. · View Herald TranscriptJul 21 2018, 11:48 AM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
vkrause requested review of this revision.Jul 21 2018, 11:48 AM
dvratil requested changes to this revision.Jul 21 2018, 12:52 PM
dvratil added a subscriber: dvratil.

In this case, the session thread is destroyed from a slot connected to QApplication::aboutToQuit() signal (see SessionPrivate ctor), so I think just checking if session->d->sessionThread() returns a valid pointer in ChangeNotificationDependenciesFactory should do the job, without having the execution to actually enter a method with invalid this pointer.

This revision now requires changes to proceed.Jul 21 2018, 12:52 PM
vkrause added a comment.Jul 21 2018, 1:45 PM
In D14263#295602, @dvratil wrote:

In this case, the session thread is destroyed from a slot connected to QApplication::aboutToQuit() signal (see SessionPrivate ctor), so I think just checking if session->d->sessionThread() returns a valid pointer in ChangeNotificationDependenciesFactory should do the job, without having the execution to actually enter a method with invalid this pointer.

That doesn't seem to work, this isn't detectable as invalid at this point,

#6 0x00000000000002a0 in ()
#7 0x00007f05e1ba0f43 in QMetaObject::invokeMethod(QObject*, char const*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) (obj=obj@entry=0x260e1c0, member=<optimized out>, member@entry=0x7f05e4f651e0 "doDestroyConnection", type=type@entry=Qt::BlockingQueuedConnection, ret=..., val0=..., val1=..., val2=..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at /data2/k/qt5/src/qtbase/src/corelib/kernel/qmetaobject.cpp:1474
#8 0x00007f05e4e61907 in QMetaObject::invokeMethod(QObject*, char const*, Qt::ConnectionType, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) (obj=obj@entry=0x260e1c0, member=member@entry=0x7f05e4f651e0 "doDestroyConnection", type=type@entry=Qt::BlockingQueuedConnection, val0=..., val1=..., val2=..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at /k/qt5/inst/include/QtCore/qobjectdefs.h:444
#9 0x00007f05e4ed36bf in Akonadi::SessionThread::destroyConnection(Akonadi::Connection*) (this=0x260e1c0, connection=<optimized out>) at /k/kde5/src/akonadi/src/core/sessionthread.cpp:85
#10 0x00007f05e4eb02d7 in Akonadi::MonitorPrivate::~MonitorPrivate() (this=0x2523690, in_chrg=<optimized out>) at /k/kde5/src/akonadi/src/core/monitor_p.cpp:66
#11 0x00007f05e4eb03f9 in Akonadi::MonitorPrivate::~MonitorPrivate() (this=0x2523690, in_chrg=<optimized out>) at /k/kde5/src/akonadi/src/core/monitor_p.cpp:64
#12 0x00007f05e4ea3a46 in Akonadi::Monitor::~Monitor() (this=0x2561050, in_chrg=<optimized out>) at /k/kde5/src/akonadi/src/core/monitor.cpp:61
#13 0x00007f05e4ea3a59 in Akonadi::Monitor::~Monitor() (this=0x2561050, in_chrg=<optimized out>) at /k/kde5/src/akonadi/src/core/monitor.cpp:57
#14 0x00007f05e1bc172a in QObjectPrivate::deleteChildren() (this=this@entry=0x2600550) at /data2/k/qt5/src/qtbase/src/corelib/kernel/qobject.cpp:1997
#15 0x00007f05e1bc2a9f in QObject::~QObject() (this=<optimized out>, in_chrg=<optimized out>) at /data2/k/qt5/src/qtbase/src/corelib/kernel/qobject.cpp:1025
#16 0x00007f05a6bdac39 in Akonadi::ETMCalendar::~ETMCalendar() (this=0x2550690, in_chrg=<optimized out>) at /k/kde5/src/akonadi-calendar/src/etmcalendar.cpp:505

dvratil accepted this revision.Jul 24 2018, 7:15 PM

Damn the raw pointer hell that I created :) Ship your version then - not crashing is more important. And I'll add cleaning up the code to my todo :)

This revision is now accepted and ready to land.Jul 24 2018, 7:15 PM
Closed by commit R165:bab79bac9706: Don't attempt to clean up when the process is about to exit (authored by vkrause). · Explain WhyJul 26 2018, 5:54 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/core/sessionthread.cpp (3 lines)

Diff 38471

View Options

src/core/sessionthread.cpp

Loading...