D8673: Sanitize signal handling in ksmserver

This revision was automatically updated to reflect the committed changes. Closed by commit R120:eb6e26197984: Sanitize signal handling in ksmserver (authored by jpalecek, committed by davidedmundson). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D867

D8673: Sanitize signal handling in ksmserver

jpalecek added a comment. In D8673#245716 , @davidedmundson wrote: > Thanks, do you have commit access? Sorry, no. BRANCH master REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma, davidedmundson Cc: broulik, gr

D8673: Sanitize signal handling in ksmserver

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. Thanks, do you have commit access? BRANCH master REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma, davidedmundson Cc: broulik, graesslin, davided

D8673: Sanitize signal handling in ksmserver

jpalecek updated this revision to Diff 32067. jpalecek added a comment. I finally got round to this. So: - this revision stores the socketpair fds in KSMServer instance, so they can be closed later. - also, it uses the modern QObject::connect syntax with pmfs instead of strings

D8673: Sanitize signal handling in ksmserver

broulik added a comment. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma Cc: broulik, graesslin, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8673: Sanitize signal handling in ksmserver

graesslin added inline comments. INLINE COMMENTS > server.cpp:615 > +QSocketNotifier* n = new QSocketNotifier(sockets[1], > QSocketNotifier::Read, this); > +qApp->connect(n, SIGNAL(activated(int)), SLOT(quit())); > + why the old connect syntax? REPOSITORY R120 Plasma Workspace REVIS

D8673: Sanitize signal handling in ksmserver

davidedmundson added a comment. > It would certainly be cleaner to close them in the destructor. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-moham

D8673: Sanitize signal handling in ksmserver

jpalecek added a comment. In https://phabricator.kde.org/D8673#164830, @davidedmundson wrote: > Do the sockets need closing in the destructor? Good point. I assumed the `KSMServer` instance is practically a singleton (being assigned to global `the_server` etc.) which is created on

D8673: Sanitize signal handling in ksmserver

davidedmundson added a comment. Makes sense. Do the sockets need closing in the destructor? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensre

D8673: Sanitize signal handling in ksmserver

jpalecek edited the summary of this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8673: Sanitize signal handling in ksmserver

jpalecek edited the summary of this revision. jpalecek added a reviewer: Plasma. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D8673 To: jpalecek, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8673: Sanitize signal handling in ksmserver

jpalecek created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY The TERM signal handling in ksmserver invokes functions which are not async-signal safe, like Qt functions and C++ destructors. Moreover,