D8561: [d_ptr] Do not harsh destroy QThread
This revision was automatically updated to reflect the committed changes. Closed by commit R161:27c0245b1715: [resources] Nicely quit threads (authored by anthonyfieroni). REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8561?vs=21830=21852 REVISION DETAIL https://phabricator.kde.org/D8561 AFFECTED FILES src/service/Resources.cpp src/service/Resources_p.h src/service/plugins/sqlite/ResourceScoreMaintainer.cpp To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. Thanks for fixing REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
anthonyfieroni updated this revision to Diff 21830. REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8561?vs=21603=21830 REVISION DETAIL https://phabricator.kde.org/D8561 AFFECTED FILES src/service/Resources.cpp src/service/Resources_p.h src/service/plugins/sqlite/ResourceScoreMaintainer.cpp To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
ivan added a comment. While I like the idea behind the patch, I'd rather have what David proposed. Namely, d_ptr is a smart pointer, but I don't think it should be this smart - creating specializations for different types' destructors should not really be a part of it. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
davidedmundson added a comment. consistent with what? REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
anthonyfieroni added a comment. In https://phabricator.kde.org/D8561#162062, @davidedmundson wrote: > Wouldn't it be cleaner to put the quit in: > > Resources::Private::~Private > and/or ResourceScoreMaintainer::Private::~Private > > (also I have no idea what the Resource thread is for, literally the only thing it does actually in the thread is sleep) > Yeah, it's clear to put in where private is QThread, but i think this way is more consistent. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
anthonyfieroni updated this revision to Diff 21603. anthonyfieroni added a comment. Correct patch version. REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8561?vs=21589=21603 REVISION DETAIL https://phabricator.kde.org/D8561 AFFECTED FILES src/utils/d_ptr_implementation.h To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
davidedmundson added a comment. Wouldn't it be cleaner to put the quit in: Resources::Private::~Private and/or ResourceScoreMaintainer::Private::~Private (also I have no idea what the Resource thread is for, literally the only thing it does actually in the thread is sleep) REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
davidedmundson added a reviewer: ivan. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 To: anthonyfieroni, #plasma, davidedmundson, ivan Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8561: [d_ptr] Do not harsh destroy QThread
anthonyfieroni created this revision. anthonyfieroni added reviewers: Plasma, davidedmundson. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY systemd makes coredumps in /var/lib/systemd/coredumps when kactivitymanagerd_plugin_sqlite.so quits coredumpctl list coredumpctl gdb PID (gdb) bt #0 0x6d63f0d0513f in raise () from /lib/libc.so.6 #1 0x6d63f0d0656a in abort () from /lib/libc.so.6 #2 0x6d63f148792e in QMessageLogger::fatal(char const*, ...) const () from /usr/lib/libQt5Core.so.5 #3 0x6d63f14991bc in QThread::~QThread() () from /usr/lib/libQt5Core.so.5 #4 0x6d63d17c6c8a in ?? () from /usr/lib/qt5/plugins/kactivitymanagerd/1/kactivitymanagerd_plugin_sqlite.so #5 0x6d63f0d07a80 in __run_exit_handlers () from /lib/libc.so.6 #6 0x6d63f0d07ada in exit () from /lib/libc.so.6 #7 0x6d63f2dd5e75 in QXcbConnection::processXcbEvents() () ---Type to continue, or q to quit--- from /usr/lib/qt5/plugins/platforms/../../../libQt5XcbQpa.so.5 #8 0x6d63f1699141 in QObject::event(QEvent*) () from /usr/lib/libQt5Core.so.5 TEST PLAN Try to nice quit a thread REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D8561 AFFECTED FILES src/utils/d_ptr_implementation.h To: anthonyfieroni, #plasma, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
