> 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
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to