Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

> On March 5, 2016, 9:27 a.m., David Faure wrote:
> > Looks good and more portable than the
> >     qunsetenv("SESSION_MANAGER");
> > which is used in many other places...
> > 
> > Not sure both connects are necessary though?
> 
> Xuetian Weng wrote:
>     This is not to disable the whole session manager thing, but just disable the restore. So session manager is still able to terminate kwalletd.
> 
> David Faure wrote:
>     Ah. Do you mean that with this code, the process is terminated more gracefully than with the qunsetenv solution which e.g. kiod or kded use (I guess they simply die with an X error when Xorg disappears). Sounds like another reason for this approach indeed, although it probably doesn't make much difference from a user's perspective [other than portability of course].
>     
>     Sounds like we should apply the same code in all these:
>     
>     kded/src/kded.cpp:682:    qunsetenv("SESSION_MANAGER");
>     kdesu/src/ptyprocess.cpp:305:    unsetenv("SESSION_MANAGER");
>     kglobalaccel/src/runtime/main.cpp:56:    qunsetenv( "SESSION_MANAGER" );
>     kinit/src/klauncher/klauncher_main.cpp:153:    qunsetenv("SESSION_MANAGER");
>     kio/src/kiod/kiod_main.cpp:89:    qunsetenv("SESSION_MANAGER"); 
>     
>     This also makes me wonder what happens to non-GUI daemons, i.e. what will terminate them when logging out... (all of the above are GUI enabled, but e.g. kio_http_cache_cleaner is core-only).
> 
> Xuetian Weng wrote:
>     Well, I doubt this method is useful for cases above. kiod/kded might not want to be exit by session manager maybe? Because when session terminates, it is still possible for application like kwrite/kate to use kio to write or open something.
>     
>     I think most of them (at least for klauncher/kded/kiod) are currently doing the right thing.
>     
>     kio_http_cache_cleaner should be managed by kio so I don't think we need to worry about it. core-only application could possible leave some garbage behind, for example: https://git.reviewboard.kde.org/r/125746/
> 
> David Faure wrote:
>     Ah, but then it's the same for kwalletd.... if kwrite uses KIO to save to an FTP directory, this might require a password, which would then require kwalletd. I see no difference between kwalletd and kiod/kded in that respect.
>     
>     But I don't even think that's a problem. Closing the session happens in several steps (asking all apps to save state, then asking all apps to prompt the user for saving open files, and only then closing all windows and quitting the apps). So I don't think klauncher/kded/kiod/kwalletd being asked to quit by the session manager would break anything, that's only the very last step after all the saving is done.
>     
>     kio_http_cache_cleaner is started by the first kio_http-using apps, but quitting it is difficult: it's a "singleton" process, so kio_http can't just make it quit on exit. It would need refcounting [difficult in case kio_http crashes] .... or indeed it could be QGuiApplication (not good for possible core-only kio usage). Tricky.

Then you would need to use things like https://git.reviewboard.kde.org/r/127125/ . But I still think there could be some order issue. I get the idea that kwalletd could probably just use the unset QSESSION_MANAGER method.


- Xuetian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127271/#review93187
-----------------------------------------------------------


On March 3, 2016, 8:34 p.m., Xuetian Weng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127271/
> -----------------------------------------------------------
> 
> (Updated March 3, 2016, 8:34 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Klapetek.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> I notice a kwalletd5 with "-session ....." in its command line started on my desktop and kwallet-pam doesn't work.
> 
> Also kwalletd is dbus activated in other cases there's no point to let session manager to restore it.
> 
> 
> Diffs
> -----
> 
>   src/runtime/kwalletd/main.cpp 740e670 
> 
> Diff: https://git.reviewboard.kde.org/r/127271/diff/
> 
> 
> Testing
> -------
> 
> kwallet-pam back to work.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160306/8aab6764/attachment-0001.html>