Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56571 --- This review has been submitted with commit b1fa2678d36f6ce215e8d8ec1d57608079f36c89 by Wolfgang Bauer to branch master. - Commit Hook On April 24, 2014, 10:28 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 10:28 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 3:42 p.m.) Review request for kde-workspace and Plasma. Changes --- Another update to reflect the changes in RR#117091 Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs (updated) - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56416 --- and this as well then. - Thomas Lübking On April 24, 2014, 1:42 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 1:42 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 10:28 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56437 --- This review has been submitted with commit 26a8ddc018d940ea1910ced6264344b520ad8405 by Wolfgang Bauer to branch KDE/4.11. - Commit Hook On April 24, 2014, 1:42 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 1:42 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
On April 23, 2014, 7:36 a.m., Martin Gräßlin wrote: would you please also adapt that for plasma-workspace repo (new master)? Yes, I will. Should I create a new review request for that, or should I just submit it? - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56240 --- On April 22, 2014, 10:41 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 10:41 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
On April 23, 2014, 7:36 a.m., Martin Gräßlin wrote: would you please also adapt that for plasma-workspace repo (new master)? Wolfgang Bauer wrote: Yes, I will. Should I create a new review request for that, or should I just submit it? The code looks pretty straight forward to me, so I don't think it's really needed to have another review request. Unless... in master we have a few auto-tests, so perhaps you want to add a test for this case? Given the mentioned number of votes it might not be the worst idea to have a regression test for it ;-) - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56240 --- On April 22, 2014, 10:41 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 10:41 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/ksldapp.cpp 3dfcc9e ksmserver/screenlocker/greeter/main.cpp d898734 Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 10:41 p.m.) Review request for kde-workspace and Plasma. Changes --- Adapted the patch to a change in https://git.reviewboard.kde.org/r/117091/ Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs (updated) - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56219 --- Ship it! from here - see veto condition in the other RR (i do not maintain the screenlocker), but SIGTERM is oc. far better than SIGKILL - Thomas Lübking On April 22, 2014, 8:41 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 8:41 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56240 --- would you please also adapt that for plasma-workspace repo (new master)? - Martin Gräßlin On April 22, 2014, 10:41 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 10:41 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
