KDE5/Qt5 does not support session restoration

Bug #1446865 reported by Laurent Bonnaud
22
This bug affects 4 people
Affects Status Importance Assigned to Milestone
KDE Base Workspace
Fix Released
Medium
plasma-workspace (Ubuntu)
Fix Released
Medium
Unassigned
qtbase-opensource-src (Ubuntu)
Fix Released
Medium
Unassigned

Bug Description

KDE5/Qt5 does not support proper session restoration.

ProblemType: Bug
DistroRelease: Ubuntu 15.10

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

When starting a new session (after reboot or logout), applications that were left open on various Activities / Desktops are all restarted on the first Activity, on the first Desktop, instead of being restarted on their previous location.

Reproducible: Always

Steps to Reproduce:
1. Place some applications on various Activities and Desktops
2. Logout or Restart
3. Login in the same session as in 1

Actual Results:
Applications are all restarted on Desktop 1 in first Activity

Expected Results:
Applications should have been restarted on the same Desktop and Activity as they were when session was quit.

Plasma Desktop 5.1.1 using Frameworks 5.5.0

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

Still there in 5.2. Is there anything I can do to help ? Anyone else noticing this ?

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

Unfortunately, this is not something that we have the control over with the X session protocol.

If we get something else in place, this might be fixed, but that is not going to happen any time soon.

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

Thanks for the feedback, Ivan. Does it mean Wayland should be preferred to X for this issue ?

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

No. The only benefit of Wayland here is that we will need to implement the sessions in a new way. Which will benefit both X and Wayland.

The problem is that non-KDE applications will probably never support this. Gnome people have decided they do not care about sessions (AFAIK).

Revision history for this message
In , Anders Lund (anders-alweb) wrote :

Confirmed.

This works in KDE 4, so how can it be different in KF 5? Same X.org afaik...
Worse, this renders activities useless. Even windows from a closed activity is opened in the wrong place.

Revision history for this message
In , Anders Lund (anders-alweb) wrote :

Ivan, what does it matter what gnome people thinks? this affects kde applications (as well).

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

> This works in KDE 4, so how can it be different in KF 5? Same X.org afaik...

Will see whether something has changed. For me, it did not work on 4 as well. Will see to bring this up to the level it had in kde 4.

Revision history for this message
In , Anders Lund (anders-alweb) wrote :

Thank you Ivan! You rock!

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Virtual desktop and activities are stored in the kwin session data - the client has no word in this.

Session management is branched in "#if KWIN_QT5_PORTING" which is currently defined 0 in kwinglobals.h (but I don't know why atm. either)

Revision history for this message
In , Mgraesslin (mgraesslin) wrote :

> Session management is branched in "#if KWIN_QT5_PORTING" which is currently defined 0 in kwinglobals.h (but I don't know why atm. either)

When I started the porting Session management was broken in Qt and made the code not compile. That's why it's in an ifdef. Apparently nobody cared so far to port it since then.

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

I can confirm it worked in 4 for me as well. Looking forward to see this back, it's one of the killer features of the Plasma Desktop, IMHO :)

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

I'll see whether it can work now.

@Thomas Lübking
Thanks for investigating this!

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

*** Bug 343843 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

Can anyone test whether the session management works in, for example, kate:
 - open a document
 - log out
 - log back in
 - kate should be open *with* that document inside

Why kate? It properly saves the session information for me, but I do not see it restored - the application gets started, but empty.

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

Hi Ivan,

I can confirm that behaviour, kate is restarted, but without the doument opened. This is still on 5.2 through Archlinux packages. Did you want us to test on master ? Cause I'm not proficient enough to build the plasma desktop from git :(

Thanks for your efforts on this !

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

before I log out: what's the commandline of the kate process that should have restored the document?

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

This is what I get from ps aux | grep kate
/opt/kf5/usr/bin/kate -session 10d372616b000142456137800000121810009_1424561394_855171

And, the file
.config/session/kate_10d372616b000142456137800000121810009_1424561394_855171
does exist

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Well, that implies the global session management is intact, but kde apparently didn't port it (correctly) just as much as kwin did not.

Revision history for this message
In , Dweeble01103 (dweeble01103) wrote :

Would this also cause my restored sessions windows to not be placed in their previous locations on the desktop but to be placed according to the initial placement as defined in systemsettings -> window behavior -> window behavior -> advanced -> placement.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

*** Bug 344856 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Graham P Davis (hacker-scarlet-jade) wrote :

Just upgraded to 5.2.2 and still got this problem. This makes Plasma-5 unusable as far as I'm concerned. Any chance of the "importance" level being shifted up a notch or so?

I can also confirm that the problem doesn't exist on the same machine and same user.

Revision history for this message
In , Graham P Davis (hacker-scarlet-jade) wrote :

(In reply to Graham P Davis from comment #21)
> Just upgraded to 5.2.2 and still got this problem. This makes Plasma-5
> unusable as far as I'm concerned. Any chance of the "importance" level being
> shifted up a notch or so?
>
> I can also confirm that the problem doesn't exist with KDE4 on the same machine and
> same user.

Inserting "with KDE4" makes a lot more sense. Deary me!

Revision history for this message
In , Loose-control (loose-control) wrote :

Any update on this?
I am also effected by this and would like to see a fix :)

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

Just upgraded to 5.2.95, and bug is still there. Any chance of seeing this fixed before 5.3 is released ? Can I help with some testing or anything else ?

Thanks :)

Revision history for this message
In , Mgraesslin (mgraesslin) wrote :

It needs someone who cares about session management (not me) to remove the ifdefs from KWin and port it to the new API in Qt 5. Given that this hasn't happened for over a year, I doubt it will happen this week to make it into 5.3.

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

I did start playing around with it, but I don't expect it to be in 5.3.

Revision history for this message
Laurent Bonnaud (laurent-bonnaud) wrote :
Revision history for this message
In , Laurent Bonnaud (laurent-bonnaud) wrote :

This bug is still there in Plasma 5.3.0 KF 5.9.0 (Kubuntu 15.04 + backports).

summary: - kwin does not remember on with desktop to open windows on session start
+ kwin does not remember on which desktop to open windows on session start
Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

*** Bug 341336 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mournblade (mournblade) wrote :

I just updated to plasma5 from kde 4.x and was shocked to see session management across desktops completely broken in what is supposed to be the third major "stable, non-beta" release. I thought, I'd learned my lesson from the premature release of 4.x and waited 9 months to try, but apparently didn't wait long enough. I've been using KDE for 8 years, but have moved to another desktop until this issue gets resolved. It's disturbing to learn that the devs have apparently lost interest in session management.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

> what is supposed to be the third major "stable, non-beta"

No, it's not.
Plasma 5 is not nearly ready for productive use and there also still major bugs in (released) Qt5 versions.

It's "reliable and usable enough™" for everyday testing (you can install it next to other desktop environments, some distros allow to aside it next to KDE4) and development on it and you're encouraged to do so, but there still be monsters (worse than this, the QScreen::handle() bug killed my ksmserver today...)

Somebody tells you different: he's lying. Period.
The numbers are marketing bs. Has been the same w/ KDE4 & KDE3.

/disclaimer: personal opinion

Revision history for this message
In , Kairo-kairo (kairo-kairo) wrote :

This is a bug reporting system, not a discussion forum. Please refrain from any "me too" comments or other stuff that doesn't help to move the actual implementation of a fix forward. Thanks.

Changed in kdebase-workspace:
importance: Unknown → Medium
Revision history for this message
In , Mgraesslin (mgraesslin) wrote :

Thanks Robert for your comment! Just to make it more clear: if I get whining here, I'm not motivated working on it. Now I also go to answer why I don't care about session management: because I do not restart the session, I restart the session whenever the system freezes, so session management is nothing which suits my workflow, nothing I could work on and get it right, because I don't even know how it's supposed to work.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

That said, I started to look into it last night.
Ivan, how far have your efforts gone so far?
Otherwise I think I can have a patch by the end of the week.

Revision history for this message
In , Ivan-cukic-s (ivan-cukic-s) wrote :

Unfortunately, I haven't spent much time on it, so feel free.

Revision history for this message
In , John M (s2) wrote :

>>if I get whining here, I'm not motivated working on it. <<

Huh. I hope you don't take it personally. I got the same response an another bug I reported. This newer generation of opensource coders seems to be very touchy... KDE is awesome thanks for working on it.

>>Now I also go to answer why I don't care about session management: because I do not restart the session, I restart the session whenever the system freezes, so session management is nothing which suits my workflow, nothing I could work on and get it right, because I don't even know how it's supposed to work.<<

You never logout??? You understand what the bug is right? We login, and windows are not where we left them, all over the place, or missing.

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

(In reply to Thomas Lübking from comment #33)
> That said, I started to look into it last night.
> Ivan, how far have your efforts gone so far?
> Otherwise I think I can have a patch by the end of the week.

Thomas, thanks so much for looking into this, and thanks Ivan for your efforts :)
If there is anything I can do to help (mostly testing, because this is way beyond my coding abilities), please ask !

Revision history for this message
In , Woko (wolfram-koehn) wrote :

Who on earth has driven the Kubuntu-maintainers to switch to Plasma 5 so early, if it's "not nearly ready for productive use" as Thomas said ?
In their currently released Upgrade to 15.04 there is no way to stick to kde 4, so people should be warned to upgrade !
Maybe this Bug is also the cause for Bug346768 I reported.
I'm using KDE for many years and am perfectly satisfied with it so thanks to all developers, it's not your fault. Kubutu messed it up.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

It's not the same bug, but related - session management somewhat "largely" changed and was not or not sufficiently ported (I ran pretty much through the code yesterday and then figured that for some reason the sessionKey() changes _very_ early during the application start, so right now kwin reloads the "wrong" session config. Got to figure where it changes tonight ;-)

So please notice that even if (optimitically: when) this bug closes, that does NOT mean session management in KF5 apps would completely work as it does in KDE SC4.
I assume the vast majority of clients needs to be ported by hand (ie. eg. konsole is oc. responsible for managign its tab layout, this is nothing the WM could even possibly do)

Revision history for this message
In , Mgraesslin (mgraesslin) wrote :

(In reply to Wolfram from comment #37)
> Who on earth has driven the Kubuntu-maintainers to switch to Plasma 5 so
> early, if it's "not nearly ready for productive use" as Thomas said ?
> In their currently released Upgrade to 15.04 there is no way to stick to kde
> 4, so people should be warned to upgrade !

that should be obvious - users should stick to LTS and AFAIK an LTS doesn't offer the upgrade. User's choice: you can have the latest and greatest (e.g. 15.04) or stay with the more stable offering.

Revision history for this message
In , Kairo-kairo (kairo-kairo) wrote :

(In reply to Martin Gräßlin from comment #32)
> because I do not restart the session, I
> restart the session whenever the system freezes, so session management is
> nothing which suits my workflow, nothing I could work on and get it right,
> because I don't even know how it's supposed to work.

Actually, it would be awesome if we'd have session management that works similar to Firefox and actually restarts your programs when the system freezes or crashes... ;-)

Revision history for this message
In , Johann-streitwieser-b (johann-streitwieser-b) wrote :

This bug is still there in Plasma 5.3.0 (Kubuntu 15.04 + backports).

When I starting a new session (only after reboot, not after idle state ), applications that were left open on various Activities are all restarted on the first Activity or get lost, instead of being restarted on their previous location.

Reproducible: Always

Steps to Reproduce:
1. Place some applications on various Activities (by me 4 application on 4 activities)
2. Restart
3. Login in

Application are on the first Activity or they are not started again.
The this issue on Kubuntu 15.04 with plasma 5.2 and 5.3 (from backport).

Hope this can help.

Revision history for this message
In , Chemobejk (chemobejk) wrote :

Created attachment 92631
update to review #123580 patch

- add proper discard command for the kwin session file
- make sure kwin doesn't leave behind temporary session files

Revision history for this message
In , Vihai (daniele-orlandi) wrote :

Importance is NORMAL?!?!

Whole session management is broken and this has "NORMAL" importance???

Revision history for this message
In , Vihai (daniele-orlandi) wrote :

Thomas Lübking, your childish behaviour is an insult to everyone who is affected by this bug, lost time, got frustrated and finally found out this kind of consideration.

Revision history for this message
In , Rdieter-math (rdieter-math) wrote :

There is constructive work being made (for example, in https://git.reviewboard.kde.org/r/123580/ ) to address this issue.

I think you both need to take a break from this bug, and let me remind everyone of:
https://www.kde.org/code-of-conduct/

Revision history for this message
In , 8p-k26-gj (8p-k26-gj) wrote :

@Daniele: Please refrain from insulting others, this is a bug tracker, not a discussion forum. There are also other more significant issues than this missing feature; frankly I don't know anyone in the core dev team that actually uses it. Nonethtless, you could have followed Stefan's example who, instead of complaining, just provided a couple of welcome patches that will be part of the next release.

Revision history for this message
In , Graham P Davis (hacker-scarlet-jade) wrote :

A fix to this was supplied in openSUSE Tumbleweed via the KDE:Frameworks5 repo. Unfortunately, it had to be withdrawn. Very disappointing for me as I'd had a week working happily with the "fixed" P5 without seeing any problems that weren't there before. Now I've had to switch back with KDE4 until the fix is fixed.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Daniele Orlandi from comment #44)
Despite nobody here really cares about those flags you're of course absolutely right:
If, as you pointed out, the "whole session management" is broken, it really doesn't matter whether KWin restores the window attributes in this disfunctional environment.
So I simply fixed that state for you.

PS:
if you just wanted to make a point- and informationless "I have an uninformed opinion" post, we invented Facebook to spare the rest of the internet this kind of spam.

PPS:
next time maybe spend 5 seconds on reading across the bugreport before posting *anything*

Changed in kdebase-workspace:
importance: Medium → Low
Revision history for this message
In , Chemobejk (chemobejk) wrote :

FYI: issued being tracked by Qt:

* WM_WINDOW_ROLE missing: https://bugreports.qt.io/browse/QTBUG-45484
* SM_CLIENT_ID missing: https://bugreports.qt.io/browse/QTBUG-46310

Revision history for this message
In , Chemobejk (chemobejk) wrote :

I have now applied on my F22 machine the kwin fix and the Qt fixes. kwin now stores the window information for KF5 apps to its session file. For most windows restoring during login works correctly, but at least for konsole I have seen that sometimes the "iconified" state is not correctly restored.

I would appreciate reviews on the Qt fixes so that we can get them merged.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

*** Bug 348566 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Chemobejk (chemobejk) wrote :

Latest QT5.5 fixes can be found here

* https://codereview.qt-project.org/#/c/113806/
* https://codereview.qt-project.org/#/c/113901/

With both fixes applied ontop of F22 window position/iconized state save *and* restore seem to work on my machine

Revision history for this message
In , Philipp-verpoort-q (philipp-verpoort-q) wrote :

I agree with Guillaume that this should be fixed, as it dramatically reduces the usability of sessions and activities.

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

(In reply to Philipp Verpoort from comment #53)
> I agree with Guillaume that this should be fixed, as it dramatically reduces
> the usability of sessions and activities.

Philipp, I am confident this will be fixed soon, thanks to the work of Stefan and others :) Just be patient so that his patches are merged and packaged for your distro.

@Stefan, please correct me if I'm wrong, and thanks for the fixes :)

Revision history for this message
In , Chemobejk (chemobejk) wrote :

FYI: all session management fixes are now available through official F22 packages:

* kwin-5.3.1-2.fc22
* qt5-qtbase-5.4.2-2.fc22

Revision history for this message
In , gothbert (omega-y) wrote :

*** Bug 349135 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Git commit 3442664609284ea201d50f6da27222c706a75c8e by Thomas Lübking.
Committed on 17/06/2015 at 22:18.
Pushed by luebking into branch 'master'.

port session management to KF5

REVIEW: 123580

M +1 -2 main.cpp
M +3 -0 main.h
M +5 -4 main_x11.cpp
M +55 -31 sm.cpp
M +0 -13 sm.h
M +8 -14 workspace.cpp
M +7 -2 workspace.h

http://commits.kde.org/kwin/3442664609284ea201d50f6da27222c706a75c8e

Changed in kdebase-workspace:
status: Unknown → Fix Released
Revision history for this message
In , dap (dap-rock) wrote :

I still have some session problem on latest Fedora 22. I selected konsole as product but recently found it's affecting kwrite too, maybe related to this ticket.

https://bugs.kde.org/show_bug.cgi?id=349481

Revision history for this message
In , bhushan (bhush94) wrote :

*** Bug 342086 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Sergio Callegari (callegar) wrote :

> I don't care about session management: because I do not restart the session, I restart the session whenever the system freezes

Under these premises, it would be a matter of clarity to state from the main KDE web page that KDE development mainly focuses on single user workflows and that the desktop support for multiuser environments can at best be casual. Would have saved most of the whining here.

> That should be obvious - users should stick to LTS and AFAIK an LTS doesn't offer the upgrade. User's choice: you can have the latest and greatest (e.g. 15.04) or stay with the more stable offering.

Unfortunately, it is not this simple. It is not LTS -> 15.04, so you stay with LTS and you are done. It is LTS -> 14.10 -> 15.04. Those who switched from LTS to 14.10 had no information at all that they would have been compelled into Plasma 5 while "there are still monsters" (in Thomas' words). Conversely, they got reassured multiple times about the fact that Plasma 5 would have been offered as a technical preview side to side with Plasma 4 until it was ready, and that the Plasma 3 -> Plasma 4 transition mistakes would have been avoided.

In any case, now that the damage is done, let's try to limit it as much as possible. Can someone help identify:

- if wily's qt 5.4.2 contains the same fixes that are in the Fedora package mentioned above and if it can be safely backported to vivid in a ppa

- what additional patches are needed on top of the kwin package contained in the kubuntu kde backports ppa, to see if a fixed package can be shipped in a further PPA

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Sergio from comment #60)
> KDE web page that KDE development mainly focuses on single user workflows
Session restorage has nothing to do with single/multiuser questions.
It's about session restarts, not about multiple sessions.

> - if wily's qt 5.4.2 contains the same fixes that are in the Fedora package
> mentioned above and if it can be safely backported to vivid in a ppa
See comment #49 and comment #52 on the Qt patches.
They're not in Qt upstream - not even reviewed.

> what additional patches are needed on top of the kwin package contained in the kubuntu kde
> backports ppa, to see if a fixed package can be shipped in a further PPA
I don't know what's in "backports ppa", but you'll need either KWin >= 5.4.0 (not yet released) or https://git.reviewboard.kde.org/r/123580/ + the forementioned Qt patches (for Qt5/KF5 clients - otherwise only Qt4/gtk/etc clients are properly restored by kwin)

For client internal issues (what especially also includes "restarting Qt5 applications" when "restoring the last session" itfp) I'm not aware of any present patches.

Revision history for this message
In , Mgraesslin (mgraesslin) wrote :

(In reply to Sergio from comment #60)
> > I don't care about session management: because I do not restart the session, I restart the session whenever the system freezes
>
> Under these premises, it would be a matter of clarity to state from the main
> KDE web page that KDE development mainly focuses on single user workflows
> and that the desktop support for multiuser environments can at best be
> casual. Would have saved most of the whining here.

The personal opinion of one KDE developer doesn't impact KDE in a way that we need to put warnings on KDE web pages. Luckily KDE is a large enough group of people that others step up and work on what they consider as important.

Revision history for this message
In , Kairo-kairo (kairo-kairo) wrote :

(In reply to Thomas Lübking from comment #61)
> See comment #49 and comment #52 on the Qt patches.
> They're not in Qt upstream - not even reviewed.

Forgive the question, but what needs to happen to get those into a Qt release? Are those steps underway? Any idea how long that usually takes?

Revision history for this message
In , Rdieter-math (rdieter-math) wrote :

See comment #52 for the upstream Qt patch reviews

Revision history for this message
In , Sergio Callegari (callegar) wrote :

> Session restorage has nothing to do with single/multiuser questions. It's about session restarts, not about multiple sessions.

True. But if you have a single monitor and a single keyboard to share between multiple users, you have people stopping and starting sessions all the time. The only condition where you do not need that is when you have a single user, who can just lock or suspend the machine when he is not using it, I guess. Session restart is the "suspend" of the poor man who has to work on a multiuser system sharing the seat.

Apart from that, thanks to Thomas and Martin for the pointers and the clarifications about the patches. Let me recap to see if I understand correctly:

1) One definitely needs commit 3442664609284ea201d50f6da27222c706a75c8e on kwin. Can it be expected to apply over stable kwin 5.3.1? If so, backporting onto kubuntu kwin should be relatively easy.

2) Furthermore, one needs to patch QT5 with:

0001-Forward-QWidget-windowRole-to-QPlatformWindow.patch

from https://bugreports.qt.io/browse/QTBUG-45484, that can be safely backported to 5.4.1 as shipped by kubuntu vivid (has already been done by a Fedora developer). And

0002-xcb-set-SM_CLIENT_ID-property.patch

from https://bugreports.qt.io/browse/QTBUG-46310. Which I do not know if is expected to apply cleanly on 5.4.1 (does anyone know btw?).

Is this all?

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Sergio from comment #65)
> > Session restorage has nothing to do with single/multiuser questions. It's about session restarts, not about multiple sessions.
>
> True. But if you have a single monitor and a single keyboard to share
> between multiple users, you have people stopping and starting sessions all
> the time.
No?

You can start several sessions simultaniously - I frankly don't know whether KDE would even allow to start a session for a new user without locking the current one.

The only problem would be if you're short on resources and/or have no swap device and/or a process of one user starts running wild.
(Might be interesting to stop all processes of the non-current user when switching sessions?)
But under normal conditions, it's no problem.

> 1) One definitely needs commit 3442664609284ea201d50f6da27222c706a75c8e on
> kwin.
yupp.

> Can it be expected to apply over stable kwin 5.3.1?
The patch can be used functionally - whether you'll run into an unresolvable patch conflict (patch not knowing how to apply this, you'll have to help), I can't say for sure (but don't expect so)

> 2) Furthermore, one needs to patch QT5 with:
> 0001-Forward-QWidget-windowRole-to-QPlatformWindow.patch
Not necessarily - the *really* important thing in this context is SM_CLIENT_ID (but it certainly won't hurt)

> from https://bugreports.qt.io/browse/QTBUG-46310. Which I do not know if is
> expected to apply cleanly on 5.4.1 (does anyone know btw?).
Trying > guessing ;-)
(But I think Stefan actually wrote it on top of 5.4.1)

> Is this all?
This will still NOT prevent Qt5 clients from not restarting at all (they somehow randomly fail to be recorded by the sessionmanager, likely killing themselves on the "session about to end" signal or whatever)

Revision history for this message
In , David Lang (david-lang) wrote :

Thomas, if you start lots of sessions without anyone logging out, you quickly run out of system resources for all the running apps.

But even with a single user on a laptop, I have to deal with the pain or restarting sessions regularly because the laptop doesn't stay plugged in all the time and doesn't have an infinite battery.

Personally, I prefer that Firefox/Chrome not startup until I tell them to because I may not have network connectivity and 40 firefox windows with a couple hundred tabs all reporting "connection failed" is not useful.

The fact that KDE4 would detect when firefox was running at shutdown (even when I start it manually from a shell window) is a bug as far as I'm concerned. Go ahead and restart apps that I started through the KDE menu, but don't try to detect apps that I start from the command line and then start them automatically (especially when you do so without the command-line parameters I provided)

Revision history for this message
In , Kairo-kairo (kairo-kairo) wrote :

(In reply to David Lang from comment #67)
> The fact that KDE4 would detect when firefox was running at shutdown (even
> when I start it manually from a shell window) is a bug as far as I'm
> concerned.

And a feature as I'm concerned.

(In reply to Thomas Lübking from comment #66)
> This will still NOT prevent Qt5 clients from not restarting at all (they
> somehow randomly fail to be recorded by the sessionmanager, likely killing
> themselves on the "session about to end" signal or whatever)

Hrm, if Qt5-based apps (like the KDE apps) don't start up by default and only old KDE4 ones do, that's an argument for not updating yet at all, sadly. Are there bugs filed for that?

(In reply to Rex Dieter from comment #64)
> See comment #52 for the upstream Qt patch reviews

Hard to make out for me how this is progressing. I hope it does.

All that said, thanks a lot to Thomas and Stefan (and anyone else helping) for the great work you have done and are doing there!

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to David Lang from comment #67)
> Thomas, if you start lots of sessions without anyone logging out, you
> quickly run out of system resources for all the running apps.

Risking a little OT, "which"?
RAM? => https://wiki.archlinux.org/index.php/Swap

The only actual problem can, as mentioned, be at some point a slowdown due to CPU overload by misbehaving processes (and while I don't think kdm or sddm or even logind support this, this could in theory be covered by stopping processes for all users that do not hold the current session)

> But even with a single user on a laptop, I have to deal with the pain or
> restarting sessions regularly because the laptop doesn't stay plugged in all
> the time and doesn't have an infinite battery.
Hibernate?

In case it's not clear, this is from my position *not* a discussion on whether session management should better work again at some point (it certainly should, for there *are* reasons to log out, eg. to reboot the kernel or shutdown to have the system fully encrypted/stashed)
It's just that multiuser environments (or battery limitations - unless you wait too long ;-) don't somhow enforce session restarts at all, so please don't try to make that your argument for working session support. It's factually wrong.

> The fact that KDE4 would detect when firefox was running at shutdown (even
> when I start it manually from a shell window) is a bug as far as I'm
> concerned.
It's actually a setting ("kcmshell5 smserver", also on SC4 for "kcmshell4", but I'm not 100% sure whether the module was named smserver than as well) and really completely unrelated.

> Go ahead and restart apps that I started through the KDE menu,
> but don't try to detect apps that I start from the command line
That's unfortunately indistiguishable.

> start them automatically (especially when you do so without the command-line
> parameters I provided)
That's actually a client bug, for it tells the smserver about the command it wants for a restart.

Revision history for this message
In , David Lang (david-lang) wrote :

>> Go ahead and restart apps that I started through the KDE menu, but don't try to detect apps that I start from the command line

> That's unfortunately indistiguishable.

only if you only do this by looking at what's running at the time you shutdown.

If something is started from the KDE menu, then you can track that you started it and the user hasn't exited it. restart those apps.

But if you are actually trying to restart all apps, then you are failing miserably. I routinely have many apps running in terminal windows that don't get restarted, as well as a few GUI apps (almost none KDE/Gnome apps) that don't get restarted. Other apps that get restarted are missing the context (what files they had open, etc) my terminal windows get started, but beyond them and the browsers, not a lot more (and currently all the terminal windws get shoved onto the first screen.

>> Thomas, if you start lots of sessions without anyone logging out, you quickly run out of system resources for all the running apps.

> Risking a little OT, "which"?
> RAM? => https://wiki.archlinux.org/index.php/Swap

swap is horrible to use, and it's still easy to run out of swap. You actually don't want to have tens of GB of swap because the system will become unusable if it is used. I run my systems with very little swap (I only have one machine with more than 2G of swap, even if the system has hundreds of GB of RAM). I've actually had a machine with 128G of RAM that only had 120G of disk. My VMs for work typically have 40G of disk or so, even if they have 32+G of RAM

disk space may be cheap, but only if you don't need to add a drive for it and aren't using expensive storage (price the per-GB cost of EMC storage for example, especially if you include replication of the storage)

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to David Lang from comment #70)
> only if you only do this by looking at what's running at the time you
> shutdown.

Nothing is tracked or something.
There's a session protocol and clients can support it and call for a restart or not.
They can notably define *how* they want to be restarted (commandline).

Even if it was not: restarting only clients that were launched through a specific interface is a pretty much instant fail. Should we care about kickoff? krunner? plasma panels? what about 3rd party dockers? Should we estrablish a protocol that each and every runner/docker/whatever needs to support? Let alone the mess that this would cause for less integrated environments as well as the most obvious question, *why* to discriminate processes that were started from eg. konsole. (Eg. I *very* often launch kwrite instances from konsole)

> But if you are actually trying to restart all apps
As pointed out: it's configurable and you're free to start with a blank session or an explicitly stored one.

> then you are failing miserably.
Thank you - But maybe better read into the topic next time.

> I routinely have many apps running in terminal windows that don't
> get restarted, as well as a few GUI apps (almost none KDE/Gnome apps) that
> don't get restarted.
As mentioned, clients either support this (dead old) protocol or not. Qt5 is apparently buggy itr. and atm.

> Other apps that get restarted are missing the context
> (what files they had open, etc)
The apps get a signal "the session is gonna end, save your stuff" and a signal "session restarted, your last id was 123456" - what they do with that is their thing and not job of the session manager.

========== OFF TOPIC ============================================

> swap is horrible to use
swap is slow, yes, but unless some process runs wild, the logged out user will quickly get swapped out in favor of the active users processes (notably when stopping processes)
Since the user isn't logged in, he doesn't care.

> I've actually had a machine with 128G of RAM that only had 120G of disk. My VMs for work
> typically have 40G of disk or so, even if they have 32+G of RAM

Sorry, but personal design decisions do not form a concept.
Naturally a multiuser (for multiseat you obviously need tons of physical RAM) system would be designed different (hard quotas and sufficient swap space to protect user processes agains other users processes causing OOM)

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Robert Kaiser from comment #68)
> Hrm, if Qt5-based apps (like the KDE apps) don't start up by default and
> only old KDE4 ones do, that's an argument for not updating yet at all,
> sadly. Are there bugs filed for that?
None that I could find, but I found https://bugreports.qt.io/browse/QTBUG-28228 which suggests that all session management was entirely uniplemented before Qt 5.2 ... :-\

Revision history for this message
Launchpad Janitor (janitor) wrote : Re: kwin does not remember on which desktop to open windows on session start

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in kwin (Ubuntu):
status: New → Confirmed
Changed in kwin (Ubuntu):
importance: Undecided → Critical
importance: Critical → Medium
Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

When open chromium, konsole, thunderbird, then logout, relogin, but only thunderbird successful WM_SAVE_YOURSELF, chromium or konsole failed to be opened.

plasma-workspace: 5.4.90
KF5: 5.15.0
Qt5: 5.5.1

Reproducible: Always

Steps to Reproduce:
1. open chromium, konsole, thunderbird, then logout
2. relogin

Actual Results:
but only thunderbird successful WM_SAVE_YOURSELF

Expected Results:
chromium and konsole able to be opened.

Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

my ~/.config/ksmserverrc
[General]
screenCount=1

[LegacySession: saved at previous logout]
clientMachine1=localhost
command1=thunderbird
count=1

[Session: saved at previous logout]
clientId1=1014cd7d2d4000144435205700000012260003
clientId2=1014cd7d2d4000144435208500000012260012
clientId3=10bd534f46000144610537700000070460007
count=3
discardCommand1[$e]=rm,$HOME/.config/session/kwin_1014cd7d2d4000144435205700000012260003_1446105607_787343
discardCommand2[$e]=rm,$HOME/.config/session/kmix_1014cd7d2d4000144435208500000012260012_1446105607_737385
program1=kwin_x11
program2=/bin/kmix
program3=/usr/lib/mozilla/kmozillahelper
restartCommand1=kwin_x11,-session,1014cd7d2d4000144435205700000012260003_1446105607_787343
restartCommand2=/bin/kmix,-session,1014cd7d2d4000144435208500000012260012_1446105607_737385
restartCommand3=/usr/lib/mozilla/kmozillahelper,-session,10bd534f46000144610537700000070460007_1446105607_737492
restartStyleHint1=0
restartStyleHint2=0
restartStyleHint3=0
userId1=lesliezhai
userId2=lesliezhai
userId3=lesliezhai
wasWm1=true
wasWm2=false
wasWm3=false

Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

When open konsole, dolphin, systemsettings5, then logout

legacy.cpp, line 113
windowSessionId(*it, leader) is NOT EMPTY!

legacy.cpp, line 214
windowWmCommand(w) is EMPTY!

tags: added: wily
Revision history for this message
In , maelcum (ahartmetz) wrote :

While debugging the problem with Konsole, I found a common problem with Qt5 applications:

QGuiApplicationPrivate::commitData() (supposed to make sure that the session is saved) calls
QApplicationPrivate::tryCloseAllWindows() which, when QGuiApplication::quitOnLastWindowClosed() is true, invokes
QApplication::quit()

which terminates the application immediately after it reports successful session saving to the session manager ksmserver.
During session shutdown, ksmserver still removes its bookkeeping data about applications that were terminated for any reason - including those that quit right after session saving. When ksmserver saves the list of existing application slightly later, konsole is gone and, while it has a session save file, that file isn't "registered" in ksmserverrc, which makes it useless.

There are at several problems:
- QGuiApplicationPrivate::commitData() effectively quits the application if the application didn't call QGuiApplication::setQuitOnLastWindowClosed(false). This is despite Qt's own documentation recommending against quitting the application due to a request to save the session data.
- setQuitOnLastWindowClosed(false) is not a good solution. Quit on last window closed is a useful feature, the problem is more that session saving implies closing windows, which doesn't make too much sense. It is supposed to prevent interaction with a state that won't be saved anymore, but there are other ways to do that, e.g. preventing user input and network event handling.
- ksmserver should maybe stop removing applications from its internal list while shutting down. Because shutdown can be aborted, changes would have to be queued and applied after an aborted shutdown.

Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

Hi Andreas,

Thanks for your analysis, I wrongly argued that it might be libSM issue, I will try your solution.

Revision history for this message
In , maelcum (ahartmetz) wrote :

Hello Leslie, just in case you were planning to change ksmserver, I've since realized that saving the session restore data of each application right away (just after receiving its session restore data) in a separate, dedicated data structure is a much better idea than messing around with the main clients list.
When session saving is aborted, simply throw that data away. No interactions to worry about.

I still don't agree with Qt's session saving behavior... but I doubt that it's fixable, for compatibility reasons.

Revision history for this message
In , Laurent Bonnaud (laurent-bonnaud) wrote :

This bug is marked as "RESOLVED FIXED" whereas with plasma 5.4.3 and Qt 5.4.2 in Kubuntu 15.10 no application is restarted on login.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

KWin is not responsible for restarting applications.
This bug was *only* about not restoring window manager attributes (position, virtual desktop, etc.) on session restorage.

There're fundamental bugs in Qt5 that prevent (both, correct and entire) restoring of Qt5 applications, see

* https://codereview.qt-project.org/#/c/113806/
* https://codereview.qt-project.org/#/c/113901/
* http://marc.info/?l=kde-core-devel&m=144832700109449&w=1

Revision history for this message
Laurent Bonnaud (laurent-bonnaud) wrote :

This bug is dependent on this upstream Qt bug:

  https://bugreports.qt.io/browse/QTBUG-45484

but launchpad would not let me enter this URL.

summary: - kwin does not remember on which desktop to open windows on session start
+ KDE5/Qt5 does not support session restoration
Revision history for this message
Laurent Bonnaud (laurent-bonnaud) wrote :

Here is a very helpful comment from Thomas Lübking from kde-bugs #341930:

KWin is not responsible for restarting applications.
This bug was *only* about not restoring window manager attributes (position,
virtual desktop, etc.) on session restorage.

There're fundamental bugs in Qt5 that prevent (both, correct and entire)
restoring of Qt5 applications, see

* https://codereview.qt-project.org/#/c/113806/
* https://codereview.qt-project.org/#/c/113901/
* http://marc.info/?l=kde-core-devel&m=144832700109449&w=1

description: updated
affects: kwin (Ubuntu) → plasma-workspace (Ubuntu)
Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

Let's wait for the latter patch to be approved and merged, and I can then cherry-pick it later.

The former patch is already part of the Qt 5.5.1 packages that will land soon to xenial.

Changed in qtbase-opensource-src (Ubuntu):
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :
Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

Hi Andreas https://git.reviewboard.kde.org/r/126311/ workaround monkey patch ;P hope you can fix it in right way ;-)

Changed in kdebase-workspace:
importance: Low → Unknown
status: Fix Released → Unknown
Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

I don't know whether there's a QTBUG reported, but the problem is that QGuiApplication tries to ::tryCloseAllWindows() in QGuiApplicationPrivate::commitData()

This is plain wrong, since the window should NOT be closed as long as the session is running.
If the session survives for some reason, you'd end up with a half-destroyed environment.

=> Window closing and process termination has to happen by the smserver once it's clear that the session is gonna end.

What the routine *wanted* to do was to send close events to all toplevel widgets (to allow clients to perform last clean-ups, data storage etc.) and check whether all events are accepted. If not, that means some client wanted some window to remain open and that should break the session termination, but without having closed half other windows and maybe some processes before.

Don't you dare to call window->closeEvent() directly, the client logic may be in some eventfilter ;-)

Changed in kdebase-workspace:
importance: Unknown → Medium
Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :
Revision history for this message
In , maelcum (ahartmetz) wrote :

In reply to comment 7: Yes, that looks like a working monkey patch :)
I'm trying to get this https://codereview.qt-project.org/#/c/142232/ merged to fix the bug properly. I'm also looking at a preliminary fix in ksmserver, but I'm not sure if I understand sub-session support, and on a related note if sub-session support as currently implemented is worth keeping. It does not seem to work very well...

Revision history for this message
In , maelcum (ahartmetz) wrote :

Note: sub-session support in the session manager is basically support for activities. Session restore of activities has never worked well enough to be useful for me.

Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

Hi Andreas,

Thanks for your patch! I do not have Qt developer's account, so I could not +1 for your great job.
Hope Qt5.x will integrate your patch to fix the restore session relative issue.

About ksmserver session management improvement, as you said make it robust http://marc.info/?l=kde-core-devel&m=144832700109449&w=1 is there someone else to work together to make ksmserver better, make KDE5 better ;-)

Thanks again for your great job!

Regards,
Leslie Zhai

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Andreas, the Qt patch kills the ability to cancel the logout process (when the process or user prevents closing a window), I'm not sure it will be accepted and if, you probably will have caused a feature breakage. See comment #8. Instead of actually closing, the system likely wants to ask whether anyone is ok with closing this window (ie. cause close events w/o actually closing the window)

Revision history for this message
In , maelcum (ahartmetz) wrote :

(In reply to Thomas Lübking from comment #13)
> Andreas, the Qt patch kills the ability to cancel the logout process (when
> the process or user prevents closing a window), I'm not sure it will be
> accepted and if, you probably will have caused a feature breakage. See
> comment #8. Instead of actually closing, the system likely wants to ask
> whether anyone is ok with closing this window (ie. cause close events w/o
> actually closing the window)

How session management works is that the application connects to the QGuiApplication::saveStateRequest() signal. When that signal arrives, the application either saves its state or calls QSessionManager::cancel() on the instance passed in the signal. Session management never works by just trying to close windows without telling the application that it was triggered by session management. In the absence of any proper mechanism, one *could* do it that way, but it's a bad idea because relevant information and actions are not available to the application that way.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

The typical fail will be the "there are 5 tabs open, close/cancel" dialogs.

Afaiu this was added because of some MS Windows behavior and even if not, one has to assume that clients simply rely on window "save your work first!" protection to be in place and not somewhen been killed by the SM (what will happen when the SM got an "ok" and then tries to close windows or sigterm the process, would it not?)

Large behavioral changes could be denied itr, sending out close events should bail you out ;-)

Revision history for this message
In , maelcum (ahartmetz) wrote :

The session manager getting an OK means asking the client and at this point the client can cancel the shutdown, or save its state and wait to get killed.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Yes, my concern is that status quo for several/many applications is that they do no connect to saveStateRequest() or commitDataRequest() and *only* perform interaction on window close events (since this is the regular case *during* the session)

Stripping this mechanism will cause a behavioral change that is prone to cause data loss on session exit - thus such change may be denied upstream. If so, the status quo can be restored by sending close events w/o causing
a) the actual loss of windows in the running session
b) the application to "accidentally" quit early

I don't discuss "what should be", but "what mess have we caused and how do we get around that" ;-)

Revision history for this message
In , maelcum (ahartmetz) wrote :

How should that be done, sending close events and expecting applications to save their state in response, but not close windows or the application? It would mean that applications are session management aware but don't use the session management API and implement a very crude version of session management. A stupid thing to do.
I think the only choice here is to break session management in applications that actually support it, or break session management in applications that don't (properly) support it, where it may or may not work semi-accidentally.
And let's face it, the only Qt applications that really care about session management and do it correctly are X applications, most of which are KDE applications. Something tells me that those are going to be fine.

Changed in kdebase-workspace:
status: Unknown → Won't Fix
Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Andreas Hartmetz from comment #18)
> How should that be done, sending close events and expecting applications to
> save their state in response, but not close windows or the application?

QCloseEvent ce;
QApplication::sendEvent(window, &ce);

You only want to emit the event, the widget doesn't close in response - the event is (usually) emitted when it wants to close.

> It would mean that applications are session management aware
No, it means they give a shit about "session management", but they do care about what happens when the user (or anything) tries to close a window.
The present code in Qt's SM tells me that this is considered the predominant way to handle data-safety - also because it's the regular incident.

Notice again that this data protection mechanism has *nothing* to do with session management in particular, eg. whenever you try to close a kwrite window w/ modified text, it will ask you "err, really? maybe safe the file before?" - and at this point the user can also say "whooops, no - I didn't want to close at all"

The present code triggers this mechanism and it might be required to preserve that.

Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 354800 has been marked as a duplicate of this bug. ***

Changed in kdebase-workspace:
status: Won't Fix → Confirmed
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in qtbase-opensource-src (Ubuntu):
status: New → Confirmed
Changed in qtbase-opensource-src (Ubuntu):
importance: Undecided → Medium
Revision history for this message
In , Alexey Chernov (4ernov) wrote :

I'm actually aware of the problem with session management since last summer, now I've upgraded my stuff to have more KF5-based applications and suprisingly found it still just doesn't work. So I've dived deeper into it this time, reading all the discussion here and last part of bug #341930, both Andreas's change proposals and the thread in the mailing-list.

I likely agree with comment #18 of Andreas and in my point of view it is the following:

1. As a whole it's a massive regression since KDE4 which affects all Qt5 applications, most of which behave correctly as a session clients. Even server parts of both KWin and KSMServer now behave correctly, thanks to Thomas's fixes, I think. But as a whole it just doesn't work at all.

2. This problem is caused by some bug in processing session management messages in Qt, which earlier wasn't a big pain and could be avoided, but due to significant changes in the whole interaction process, in the API etc. now it can't be avoided and lead to (1).

3. There's initial change (https://codereview.qt-project.org/#/c/142232/) by Andreas, which perfectly fixes the problem with any observable problems. It also fixes a fault in the session management protocol implementation for at least two OSs, which is good for Qt itself.

4. There could be potentially affected client applications which: a) were already been ported to/written for Qt5; b) process some valuable data which shouldn't be lost; c) would like to use session management to prevent loss of unsaved data; d) still don't care to follow session management protocol correctly and just exploit old hacks and errors in its implementation, which exist historically, but now is moved to a new place. Unfortunately, this term is a little objectless since it wasn't mentioned any real-life application like this.

5. I completely don't like the proposed way to preserve the compatibility with (4) and make the use case of broken session management client implementation legal and default, but also try to allow proper-written apps to still survive somehow, adding some strange workarounds to Qt as closing all the windows, but not too much, or API properties to enable proper processing of SM messages.

To sum it all up, I've applied the patch (3) and have all the session management things back again without any other changes to KDE or whatever, it's already released versions (KF-5.18.0, Plasma-5.5.3, applications-15.12.1). I'll also test Windows behaviour with some toy application. Unless any problems arise, I see no reason why this tiny and simple (and right) fix isn't applied and merged.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

> 5. I completely don't like the proposed way to preserve the compatibility with (4) and make
> the use case of broken session management client implementation legal and default, but
> also try to allow proper-written apps to still survive somehow, adding some strange
> workarounds to Qt as closing all the windows, but not too much, or API properties to enable
> proper processing of SM messages.

No ofense, but what you "like" is completely irrelevant.
You propose to intentionally break clients by library changes in some minor update, to teach developers to do right, but while you might aim their face, you're gonna hit the users (and probably yours)

We had that (I kindly remind of the qDeleteAll fix ...) and it cooked up hell.

commitDataRequest hardly shows up in lxr.kde.org, what means it's probably not used at all and aboutToQuit (which isn't used but could come to rescue) isn't used too much either.

The BY FAR! omnipresent pattern is to listen to queryClose() which is called/emitted on -guess what- close events from KMainWindow.
And that's for pretty much sure why the (wrong) behavior in QSessionManager exists.

Is this behavior correct? No.
Does this matter? NO!

It's ok to spam a #warning that this behavior is shit and deprecate and kill it for Qt6 and we might even bail out (aka "fix") KMainWindow applications NOW by invoking queryClose() on QGuiApplicationPrivate::commitData() but regardless, we MUST assume this to be a global default pattern that applications (also beyond KDE) rely on (also because it's absolutely natural to intercept closing to save data and not think of closing on session end could be something entirely different - actually the illegal behavior happens to be the most sane one...)

Now, *actually* closing windows to test interaction on session end is of course just as wrong - if the user cancels the logout by such incident, we should not have closed random other windows before (letting alone that it causes this but) - therefore I frankly do not understand what's so complicated about just faking a close event to serve the present "save your stuff" pattern in a majority in clients without causing the destructive close itself which may not only be a bit premature, but also triggers this bug.

It's the least invasive solution that does not require everyone to signal "yes, i can sessionmanagement" (what's not gonna happen) and we don't risk loosing the users data (or breaking the ability to cancel the logout)

Revision history for this message
In , Alexey Chernov (4ernov) wrote :
Download full text (5.8 KiB)

(In reply to Thomas Lübking from comment #22)
> > 5. I completely don't like the proposed way to preserve the compatibility with (4) and make
> > the use case of broken session management client implementation legal and default, but
> > also try to allow proper-written apps to still survive somehow, adding some strange
> > workarounds to Qt as closing all the windows, but not too much, or API properties to enable
> > proper processing of SM messages.
>
> No ofense, but what you "like" is completely irrelevant.

Comments like this clearly don't help the discussion or solving the problem, especially when you start your reply with them. I won't answer you the same style, but given that it's not the first one from you, my earnest request to you is to please respect each other and avoid such comments in future. In case of any questions feel free to consult https://www.kde.org/code-of-conduct/. Thank you.

> You propose to intentionally break clients by library changes in some minor
> update

Never mentioned minor update or particular version. Please don't distort.

> to teach developers to do right

No intention to do it, but any specs probably means something like that.

> but while you might aim their face, you're gonna hit the users (and probably yours)

Users were already hit when the significant part of functionality important for someone's every day use case is broken. I just can't get why it's OK to break everything for one part of users and ultimately save broken implementation to preserve some ephemeral compatibility for another. That's probably the biggest question for me in this thread. Maybe I'm wrong and those who use sessions are somewhat less important than users that sometimes save their data on quitting? It's worth mentioning then, and I'll immediately give up.

> We had that (I kindly remind of the qDeleteAll fix ...) and it cooked up
> hell.

Still better than a couple of API methods like "enableSpecifiedBehaviour()" or deleting and trying to catch SIGSEGV, right?)

> commitDataRequest hardly shows up in lxr.kde.org, what means it's probably
> not used at all and aboutToQuit (which isn't used but could come to rescue)
> isn't used too much either.
>
> The BY FAR! omnipresent pattern is to listen to queryClose() which is
> called/emitted on -guess what- close events from KMainWindow.
> And that's for pretty much sure why the (wrong) behavior in QSessionManager
> exists.

If it wasn't done before for some reason, it's better to just fix the applications, especially given that you don't need any changes in Qt to have just the same functionality with the new approach. If it's still too much to change while porting to Qt5/KF5, I really wonder what porting is.

Once again: we all could already apply the fix of Andreas and be busy fixing the necessary applications rather than keep discussing here.

> Is this behavior correct? No.
> Does this matter? NO!
>
> It's ok to spam a #warning that this behavior is shit and deprecate and kill
> it for Qt6

On the Qt6 release you would say that everyone already rely on the workaround there was in Qt5 etc. etc. That's an endless story. By the way, do you really think it's so much major change that...

Read more...

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :
Download full text (3.5 KiB)

(In reply to Alexey Chernov from comment #23)

> Comments like this clearly don't help
Seriously, you asked for breaking clients because that's what you'd "like" to do - what did you expect to hear? That's simply not an acceptable stance.

> Never mentioned minor update or particular version. Please don't distort.
So you meant to schedule this for Qt6?

> Users were already hit when the significant part of functionality important
> for someone's every day use case is broken.
Let's be honest: session restorage is apparently relevant for only a minority of users.
Loosing your data is however relevant for everyone. And the latter is the by far more severe issue. Restarting applications is merely an annoyance, loosing your work is truely expensive.

Also there's absolutely NO reason why we should not care about both - except that you'd "like" to break client code and risk data loss for some reason that completely escapes me.

> Still better than a couple of API methods like "enableSpecifiedBehaviour()"

I fully agree on that proposal to be of little help - it will be mostly ignored or used w/o accounting the implications.

> Once again: we all could already apply the fix of Andreas and be busy fixing
> the necessary applications rather than keep discussing here.

It does NOT only affect KDE applications, there're hundreds of Qt applications which might have adopted this pattern - or simply don't care about session management itfp.
Also the proper order is to fix and roll out clients, *then* remove the deprecated upstream code. That's why "=> Qt6" for this approach.

> On the Qt6 release you would say that everyone already rely on the
> workaround there was in Qt5 etc. etc.

No. Because you would tell people during Qt5 don't do this and don't rely on it because it's not gonna work with Qt6, so that when things are ported to Qt6, client code has to be fixed.
Breaking it now and depending client behavior on whether it's linked against Qt 5.6 or Qt 5.7 is plain wrong and begging for trouble.

> I just kindly remind your description of current Plasma 5 and it's
> application state: https://bugs.kde.org/show_bug.cgi?id=341930#c30.
Off topic? This was a global statement. Session management in particular is a different thing:
few people seem to really care about restoring sessions.

> In a couple of years
We'll have seen Qt6 and this removed, but even if not - it doesn't matter.
The QGuiApplication code will have a "// TODO Qt6" comment and the client code does not care about why there's a close event (which might be rejected, thus not causing eg. deletion anyway)

> The only arguable point is whether it's safe to have it fixed now or should
> another (possible API-changing) workaround should be added instead.

No.
Actually I propose to fix the "workaround" already present in QGuiApplication by turning "close()" into just sendind a close event (for that's actually the desired action) and by this fix session storage and maintain data protection with the present approaches.

Breaking that behavior may happen for Qt6, anything else will be perceived as regression.

On a sidenote:
QGuiApplication::commitDataRequest() may be the "preferred" hook, but there's...

Read more...

Revision history for this message
In , Alexey Chernov (4ernov) wrote :
Download full text (7.5 KiB)

(In reply to Thomas Lübking from comment #24)
> (In reply to Alexey Chernov from comment #23)
>
> > Comments like this clearly don't help
> Seriously, you asked for breaking clients because that's what you'd "like"
> to do - what did you expect to hear? That's simply not an acceptable stance.

No one here presents any absolutely true point, otherwise there were no discussion. I just wrote my point of view and emphasized that it's just mine and not some ultimate truth.

> > Never mentioned minor update or particular version. Please don't distort.
> So you meant to schedule this for Qt6?

No. I just stated that I didn't mention any particular version, no other implications. As to your question, I'd prefer properly test the patch, including success scenario for the default not-aware-of-session-management application, and release it as soon as possible.

> > Users were already hit when the significant part of functionality important
> > for someone's every day use case is broken.
> Let's be honest: session restorage is apparently relevant for only a
> minority of users.

According to what? Your assumption? It's not too evident for me, sorry.

> Loosing your data is however relevant for everyone. And the latter is the by
> far more severe issue. Restarting applications is merely an annoyance,
> loosing your work is truely expensive.

Hey, how could session management be "apparently relevant for only a minority of users", but fixes in its behaviour be crucial for a lot of them? Don't you contradict with yourself in these two points?

Anyway, it's very subjective and I wouldn't argue on what's more important. I agree that data loss is the worst thing which could happen. I just think it doesn't mean it should result in some messy API or library code when someone's relying on the undocumented side-effects. Just because it will surely lead to more bugs and more data loss in the future. It's just the bugs which should be fixed either in library and its clients. It's better to fix them when no one really relies on the stability too much. It looks like this time is now for KF5-based application and environment.

> Also there's absolutely NO reason why we should not care about both - except
> that you'd "like" to break client code and risk data loss for some reason
> that completely escapes me.

No, that's just postponing and messing up the whole problem. If, as you stated, almost no one implemented easy and pretty simple interaction appeared in Qt5, even less would care of possible bugs and corner cases of the workaround, more complex protocol with close event you propose. There would be just another argument that it's just too messy, not to mention already existing argument that no one uses session management.

> > Still better than a couple of API methods like "enableSpecifiedBehaviour()"
>
> I fully agree on that proposal to be of little help - it will be mostly
> ignored or used w/o accounting the implications.

Ok.

> > Once again: we all could already apply the fix of Andreas and be busy fixing
> > the necessary applications rather than keep discussing here.
>
> It does NOT only affect KDE applications, there're hundreds of Qt
> applications whic...

Read more...

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :
Download full text (7.2 KiB)

(In reply to Alexey Chernov from comment #25)

> According to what?
According to "This is not fixed in years and each and every session management code was ported as "#if 0""
If there was some relevant interest, it would be fixed long time, since it's really not that hard.

> > Loosing your data is however relevant for everyone. And the latter is the by
> > far more severe issue. Restarting applications is merely an annoyance,
> > loosing your work is truely expensive.

> Hey, how could session management be "apparently relevant for only a
> minority of users", but fixes in its behaviour be crucial for a lot of them?
[...]
> Fully agree here, but we should confirm that nobody said in the beggining
> that upstream changes were about to break session management for KF5
> applications. It was just broken.

Errr... what?
Session management (in terms of "please restore the desktop as I left it") doesn't seem very important, but if you click "logout" and *booom*, gone is your work of the last two hours because the application had no chance (or, well, listened to the wrong events) to ask you to save it, that's pretty important...

We're apparently talking past each other.
There're two steps:
a) logout, clients ask to save your stuff. That works (because of the close event)
b) login, clients should restart. That's broken (because the close event is not just an event, but the window "illegally" being withdrawn during logout)

You propse to fix (b) by breaking (a) and I'm trying to convince you that this is a really bad idea.

> bugs which should be fixed either in library and its clients. It's better to
> fix them when no one really relies on the stability too much. It looks like
> this time is now for KF5-based application and environment.
Yes, we should fix KMainWindow now (if faking close events is finally not considered a permanent behavior despite the majority of clients will probably do that in return to the data commit request - with a fair share actually just calling close() ...) but that has absolutely no implications on whether it's ok to easily break away from established (even though maybe wrong) behavior.

> No, that's just postponing and messing up the whole problem. If, as you
> stated, almost no one implemented easy and pretty simple interaction
> appeared in Qt5, even less would care of possible bugs and corner cases of
> the workaround, more complex protocol with close event you propose. There
> would be just another argument that it's just too messy, not to mention
> already existing argument that no one uses session management.

Sorry, but I really cannot read any sense into this paragraph.
Please try to rephrase it. The above isn't English grammar at all.

> It won't be fixed until it's broken
So you demand to jeopardize userdata because otherwise code won't be fixed.
Sorry, but there's no way you're ever gonna convince me in this.
Any solution that builds upon "jeopardize userdata" is not a solution at all. It's malicious.

> I'm pretty sure there would be packages that would require just
> most recent Qt version, and it would be acceptable.
And jeopardize userdata. What exactly should this help in this case?

> What's wrong in...

Read more...

Revision history for this message
In , maelcum (ahartmetz) wrote :

You can't just send fake close events to clients that don't expect that. That... technique... is a KDE specialty. KDE applications are written to deal with it. In the general case, though, it is legitimate to start destroying internal data structures in a close event, and it is legitimate not to expect more than one close event before the window is actually closed. Case in point: fixing KMainWindow and KApplication to restore their KDE4 behavior (I have locally tested Qt and KDE patches to that effect) makes Kate crash on logout.

Changing behavior but not API is *worse* than adding API that optionally changes behavior - it silently breaks expectations of existing software.

We can, however, implement a workaround in KDE (and then fix our stuff when something breaks):
At the end of the slot handling commitDataRequest(), install an event filter on the QGuiApplication, which nicely filters ALL events to everything (TODO: check that - otherwise we'd just install an event filter on all toplevel windows). In that filter, check whether QGuiApplication::isSavingSession() is still true: if so, filter out and ignore() all CloseEvents. If not, have the the filter uninstall and delete itself for performance reasons. If you look at QWidgetPrivate::close_helper(), you see that it always sends a close event to ask windows if they agree to be closed, and they can always refuse.

Now which repository should that go in? It would be ugly to copy and paste the necessary code around - it should be roughly ten lines.

Revision history for this message
In , Alexey Chernov (4ernov) wrote :
Download full text (11.2 KiB)

(In reply to Thomas Lübking from comment #26)
> (In reply to Alexey Chernov from comment #25)
>
> > According to what?
> According to "This is not fixed in years and each and every session
> management code was ported as "#if 0""
> If there was some relevant interest, it would be fixed long time, since it's
> really not that hard.

Rather interesting indicator. Why don't you apply it to Qt5 or KF5 then? What a selective vision :)

> > > Loosing your data is however relevant for everyone. And the latter is the by
> > > far more severe issue. Restarting applications is merely an annoyance,
> > > loosing your work is truely expensive.
>
> > Hey, how could session management be "apparently relevant for only a
> > minority of users", but fixes in its behaviour be crucial for a lot of them?
> [...]
> > Fully agree here, but we should confirm that nobody said in the beggining
> > that upstream changes were about to break session management for KF5
> > applications. It was just broken.
>
> Errr... what?
> Session management (in terms of "please restore the desktop as I left it")
> doesn't seem very important, but if you click "logout" and *booom*, gone is
> your work of the last two hours because the application had no chance (or,
> well, listened to the wrong events) to ask you to save it, that's pretty
> important...

Hmmm... so session management "doesn't seem very important", but there're applications which expect a) to be closed gently, and also b) to have an opportunity to interact to the user the very special way, so that the rest of the world is waiting for them and doesn't logout, but it's surely not session management. Nice. Following this way we have, I think, thousands of apps which don't use X, kernel etc. and other not so important stuff.

> We're apparently talking past each other.
> There're two steps:
> a) logout, clients ask to save your stuff. That works (because of the close
> event)
> b) login, clients should restart. That's broken (because the close event is
> not just an event, but the window "illegally" being withdrawn during logout)
>
> You propse to fix (b) by breaking (a) and I'm trying to convince you that
> this is a really bad idea.

The matter is just that if you like the fruitful results of some service or protocol, you need to follow the rules of it. If you violate them and currently it just works, it's natural that anyone can change something internally and you are going to fail. Rather atomic thing.

My proposal is just to have library code of Qt following the proper interaction process, which is expected by anyone who haven't read this discussion and just wants to support session management in the application. Nothing against any workarounds in KSMServer, KF wrappers or anywhere else downstream.

> > bugs which should be fixed either in library and its clients. It's better to
> > fix them when no one really relies on the stability too much. It looks like
> > this time is now for KF5-based application and environment.
> Yes, we should fix KMainWindow now (if faking close events is finally not
> considered a permanent behavior despite the majority of clients will
> probably do that in return to the data commit re...

Revision history for this message
In , maelcum (ahartmetz) wrote :

We cannot change Qt in a way that breaks existing applications. Qt5 has not exactly just been released, and commercial customers value stability very much. Some of them even pay for Qt licenses, which is good for all Qt users, so really, we should not make things worse for them.

Revision history for this message
In , Alexey Chernov (4ernov) wrote :

(In reply to Andreas Hartmetz from comment #29)
> We cannot change Qt in a way that breaks existing applications. Qt5 has not
> exactly just been released, and commercial customers value stability very
> much. Some of them even pay for Qt licenses, which is good for all Qt users,
> so really, we should not make things worse for them.

The same way commercial customers or applications would be affected with API changes. I think, this issue (and fix) more concerns the environment than the application itself.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Alexey Chernov from comment #30)
> The same way commercial customers or applications would be affected with API
> changes.

How an ABI styable API extension could affect anyone is frankly beyond me - I doubt it will help to resolve the problem but there's really no problem with it.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Andreas Hartmetz from comment #27)

> We can, however, implement a workaround in KDE (and then fix our stuff when
> something breaks):
> [...]
> Now which repository should that go in? It would be ugly to copy and paste
> the necessary code around - it should be roughly ten lines.

That's some sort of problem.
KXmlGui/KMainWindow would cover most™ cases, but certainly not all.

The idea of KF5 is to merely optionally extend Qt and the QPA plugin would affect every Qt application.
Since this will however also require to fix the application wrt listening to the datacommit request itfp, this could only apply to the fixed cases - which would for a general fix then be KMainWindow, calling queryClose() on this occasion.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

For a KMainWindow solution, one should not require a nasty global eventfilter on the application - handling KMainWindow::closeEvent() should be sufficient, but there might be an additional pitfall on modal windows (ie. if there's already a save dialog, we might have to forcefully activate that to cause user interaction)

However, as long as QGuiApplication cancels the session lougout as long as any close event is ignored, this cannot be applied either (ie. still requires your new patch to Qt) and, of course, this doesn't provide a solution for Qt5 either - session management on Qt5 will remain broken forever and just some KDE applications work around that. That's a pretty crappy situation :-(

Revision history for this message
In , maelcum (ahartmetz) wrote :

Yes indeed, it doesn't work because ignoring close events cancels logout. Damn.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

How exactly did you try the kwrite crashing workaround? Just by sending a zombie closeEvent?
Do you still have a backtrace?

(Let's say it's legit for a leaf widget to assume that the close event it doesn't ignore() will cause a close with all implications on future user interaction, data deletion and stuff, but it's not necessarily good style.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Fun fact (though you likely already know) - KApplication::commitData(.) *did* send a fake close event to everything but KMainWindow ...

Revision history for this message
In , maelcum (ahartmetz) wrote :

Created attachment 96913
Fix session saving / KApplication changes

Revision history for this message
In , maelcum (ahartmetz) wrote :

Created attachment 96914
Fix session saving / KMainWindow changes

Revision history for this message
In , maelcum (ahartmetz) wrote :

These patches mostly fix session saving (and therefore restoring), together with the necessary Qt patch. Applications not using KApplication or KMainWindow will need to call QSessionManager::setAutoCloseWindowsEnabled(false) themselves. There a some processes like that in your average KDE session but the patches are trivial so I'm not posting them.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Please file a review request (attaching the frameworks group), the bug is assigned to Seli and I'm not sure anybody but us reads it.
I can already say that it lacks a QT_VERSION test.

About the Qt side, one might want to consider using a Qt::ApplicationAttribute instead?
"Qt::AA_DoNotKludgeSessionSaving"?

Revision history for this message
In , maelcum (ahartmetz) wrote :

Those patches are just what I currently have, they are just intended to show the important logic changes. I wasn't really planning to even submit them for review because unfortunately I seem to be the expert on session management.
It seems pretty clear that applications either largely expect KDE4 behavior through old APIs, which the patches restore, or they don't but still expect not to get killed for no good reason. The solution there is clear as well.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

So, looking into the Qt sources a bit more it seems that

a) Session management is FUBAR in Qt5 and actually was likewise FU in Qt4 - adding the proposed flag will only restore the completely FU condition of Qt4, ie. allow clients (KDE) to workaround the brokeness ...
b) this actually *only* seems to affect the xcb platform ("GuiApplicationPrivate::commitData()" is only called by "QPlatformSessionManager::appCommitData()" which is only called by "sm_performSaveYourself(QXcbSessionManager *sm)" in QXcbSessionManager)

Restoring the Qt4 state however won't cut it: KApplication is deprecated and there's no Guarantee for a KMainWindow around. Also every "plain" Qt5 client that can be found on KDE users desktops (qupzilla, trojita, ... whatever) won't be covered.

If you "seem to be the expert on session management" we need to define a roadmap out of this mess, or it's never gonna happen.

What do you think about this process:

1. emit commitDataRequest()
2. at this point either the logout should be canceled (requires API addition on QSessionManager to wire up sm_cancel; internal requirement only though. Maybe kept in d_ptr contexts) or there thould be no windows with QWidget::isWindowModified() or modal transients around since that indicates some user interaction should have taken place but didn't
3. If we detect missing interaction, spam a QMessageBox and inform/ask the user:
   "It looks like the application should have intercepted the logout request, but didn't. Do you wish to cancel the logout and explicitly save data? Since this is an application bug and this failsafe check will be removed with Qt6, please file a bug against the application."
4. If this still didn't cancel the logout, start closing windows depending on an application attribute (see below) and a trumping environment variable QT_CLOSE_WINDOWS_ON_LOGOUT_REQUEST (to pass the user ultimate control over the behavior)
5. Remove attribute, failsafe check and (actually even documented, we can NO WAY just remove this in Qt5) window closing in Qt6

I'd go for the application attribute since indeed there's little point in adding functions to implement a short-term workaround.
Actually, I might even be in favor of a dynamic property (QObject::setProperty()) since it will spare the clients compile time checks - we an add this to KFooBar now, even though Qt 5.6 (or whatever) is not on the horizon.

Revision history for this message
In , Guillaume-debure-9 (guillaume-debure-9) wrote :

Guys, I just wanted to say a big thanks for looking into this :). Being the original reporter of related bug #341930, session management in *very* important for me.

While your technical discussion is way far above my technical abilities, if there is anything I can do, like testing patches, please feel free to ask !

Thanks again :) :) :)

Revision history for this message
In , maelcum (ahartmetz) wrote :

I don't think that either dynamic properties or changing behavior that has been pretty much proven to be not broken by being around for over 10 years with no complaints will fly upstream, and I don't think they are a very good idea myself.
For Qt5, an application attribute might be a good idea.
For Qt6, I don't know. I mean there is the small problem that pseuso-SM for applications that think ignoring SM is fine only works if it asks absolutely nothing from applications. An application attribute might even work there as well: if you do care about SM, you have let's say at least 20 lines to write so you can live with another trivial one.

Revision history for this message
In , maelcum (ahartmetz) wrote :

..and frankly, I don't feel like gold-plating the solution to this mess. It's not going to be pretty either way, nobody cares too much except when their stuff breaks (ours did), and there are many people to convince to effect really big changes. There is bigger fish to fry.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Well, "shrug, I don't care then" - Qt seems to be dropping desktop support anyway, so it probably doesn't really matter.

For principal reasins I'd however object " proven to be not broken by being around for over 10 years":
It wasn't "broken" because one major client class (KDE) simply replaced that "unbroken" code.

Nor did I mean to support directly "changing behavior" - I simply suggested to raise awareness of the problem by intercepting "this looks unimplemented" conditions to allow getting rid of the "unbroken" code in Qt6 w/o pain.

Closing windows certainly is wrong (and apparently wasn't done before Qt4, seems more like a closeEvent was faked previously) because it alters the session even on a canceled session exit, regardless on whether it breaks restoring them (which one would kludge away in the session manager)

Ok, bottom line is SM is FUBAR on Qt5 and there's no intention to ever fix that, merely re-allow client code to bypass the broken Qt behavior. I'll tag future bugs respectively.

Revision history for this message
In , maelcum (ahartmetz) wrote :

Git commit 58e49487aece3de19aae90bbb9b80cd5aab94d04 by Andreas Hartmetz.
Committed on 19/02/2016 at 18:55.
Pushed by ahartmetz into branch 'master'.

Fix session management for KApplication based applications.

- Call QGuiApplication::setFallbackSessionManagementEnabled(false)
  to prevent premature application exit
- Wire up the saveStateRequest() and commitDataRequest() signals
  to the appropriate methods that had to be turned into slots first.
  Those methods were never even called, they were not ported properly.
- Cancel logout when the user decides to do that. A comment in the
  code was not sufficient to do that. (?!?!)

M +16 -1 src/kdeui/kapplication.cpp
M +15 -14 src/kdeui/kapplication.h

http://commits.kde.org/kdelibs4support/58e49487aece3de19aae90bbb9b80cd5aab94d04

Revision history for this message
In , maelcum (ahartmetz) wrote :

Git commit f7cbcc77722256db084d3b0ab6ce76173e959f0e by Andreas Hartmetz.
Committed on 19/02/2016 at 18:49.
Pushed by ahartmetz into branch 'master'.

Fix session management broken since KF5 / Qt5.

Requires Qt 5.6 branch not more than a few days old, or >= 5.6.0
when it is released.
Parts of the fix are:
- Call QGuiApplication::setFallbackSessionManagementEnabled(false)
  to prevent application suicide through a mechanism that tries to
  help applications without any proper session management support,
  but badly interferes with applications that do implement proper
  session management, such as KDE applications.
- Add back commitData[Request] handling. For some reason it was
  removed during porting.
- Change the returned types of saveState() and commitData() to void.
  The return values were unused.

M +41 -3 src/kmainwindow.cpp
M +2 -1 src/kmainwindow_p.h

http://commits.kde.org/kxmlgui/f7cbcc77722256db084d3b0ab6ce76173e959f0e

Revision history for this message
In , maelcum (ahartmetz) wrote :

Git commit a08befeac43647e222f48dfd7bed067be81573c4 by Andreas Hartmetz.
Committed on 19/02/2016 at 19:08.
Pushed by ahartmetz into branch 'master'.

KNotes: fix session save / restore.

Requires Qt >= 5.6.0 or recent 5.6 branch.

M +3 -0 knotes/src/apps/knotesapp.cpp

http://commits.kde.org/kdepim/a08befeac43647e222f48dfd7bed067be81573c4

Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 349432 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 357942 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 343518 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 355707 has been marked as a duplicate of this bug. ***

Changed in kdebase-workspace:
status: Confirmed → Fix Released
Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 357641 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Hewanci (hewanci) wrote :

Just upgraded to 5.6.0, it is NOT fixed, but quite to the contrary, session restore is now even more broken than before.

Until now Firefox and Cairo-Dock were restored, but not anymore.

Please reopen!

Revision history for this message
In , Hewanci (hewanci) wrote :

Sorry, forgot details:

Arch Linux 64 bit
qt5 5.6.0
plasma 5.6.0
plasma-framework 5.20.0

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

Firefox or cairo-dock were certainly not targetted or affected by the related patches.

This only affects Qt applications directly, so if that indeed triggered something, either the session crashes on logout or the mentioned applications used some timer based self-quitting (and are thus no more alive when the session ends)

Revision history for this message
In , Hewanci (hewanci) wrote :

(In reply to Thomas Lübking from comment #57)
> Firefox or cairo-dock were certainly not targetted or affected by the
> related patches.

I did some more testing, and indeed Qt apps (or at least most of them) seem to get restored properly now. The following get restored:
 - Dolphin
 - Konversation
 - Konsole
 - Ksysguard
 - KDE system settings

The following do NOT get restored:
 - Firefox
 - Google Chrome
 - VLC Media Player
 - Cairo-Dock

Isn't Chrome using Qt tho? I don't know.

And I have no idea what may lie beneath this issue, I'm just a noob and all I can do is telling what I experience.

I have another issue which may be related, but the two are spanning across different time frames so it doesn't seem likely. This issue is shutdown being halt with "A stop job is running for session c2 of user" with a 1:30 timeout counter. However, session restore was broken ever since I use Plasma/KDE 5, which is more than a year I think, while this issue only started 1-2 months ago. Also, the "stop job is running" issue appears randomly, while session restore always failed consistently.

Revision history for this message
In , Thomas-luebking (thomas-luebking) wrote :

(In reply to Storm Engineer from comment #58)

> The following do NOT get restored:
> - Firefox

https://forum.kde.org/viewtopic.php?f=111&t=131515

> Isn't Chrome using Qt tho?
No. And as mentioned, Qt (vlc) still *is* broken itr.
It by this patch just got the *option* to not act brokenly.
Every client needs to opt into that. (But that's not been different with Qt4, just no additional Qt API was required then and kdelibs fixed it for most KDE applications implicitly)

> I have another issue which may be related, but the two are spanning across
> different time frames so it doesn't seem likely. This issue is shutdown
> being halt with "A stop job is running for session c2 of user" with a 1:30

Seems a systemd bug, see https://bugs.freedesktop.org/show_bug.cgi?id=70593 or https://bugs.freedesktop.org/show_bug.cgi?id=70593

It might be related to firefox session management, you could try whether you also get this problems when quitting all applications (ensure there're no stale processes left!) before exitting the session and then (if it's now gone) see whether adding some process back at the session end (ie. leave dolphin running, try shutdown; then leave firefox for the next shutdown) causes this. The process may not quit on sigterm.

Revision history for this message
In , Hollense (hollense) wrote :

Just wanted to say thank you.
I've got ~10 articles open in okular at a given time,
and since it has no session management of its own
this fix was most appreciated.
Wish I could beam you a beer.

Revision history for this message
In , Leslie Zhai (xiangzhai83) wrote :

Yup also worked for ArchLinux now ;-)

Revision history for this message
In , Piotr-mierzwinski (piotr-mierzwinski) wrote :

(In reply to Storm Engineer from comment #58)
> (In reply to Thomas Lübking from comment #57)
> > Firefox or cairo-dock were certainly not targetted or affected by the
> > related patches.
>
> I did some more testing, and indeed Qt apps (or at least most of them) seem
> to get restored properly now. The following get restored:
> - Dolphin
> - Konversation
> - Konsole
> - Ksysguard
> - KDE system settings
>
> The following do NOT get restored:
> - Firefox
> - Google Chrome
> - VLC Media Player
> - Cairo-Dock
>
> Isn't Chrome using Qt tho? I don't know.
>
> And I have no idea what may lie beneath this issue, I'm just a noob and all
> I can do is telling what I experience.
>
> I have another issue which may be related, but the two are spanning across
> different time frames so it doesn't seem likely. This issue is shutdown
> being halt with "A stop job is running for session c2 of user" with a 1:30
> timeout counter. However, session restore was broken ever since I use
> Plasma/KDE 5, which is more than a year I think, while this issue only
> started 1-2 months ago. Also, the "stop job is running" issue appears
> randomly, while session restore always failed consistently.

The problem not restored GTK+ applications in Plasma 5.6.x (built with Qt 5.6.x) was cased removing support for XSM protocol: https://quickgit.kde.org/?p=plasma-workspace.git&a=commit&h=5f0ca1305db4a925dbdbf927f541497be334feff
https://bugs.kde.org/show_bug.cgi?id=362671
I reported bug related with restoring GTK+ applications, and before couple of days it has been restored. Check this report:
https://bugs.kde.org/show_bug.cgi?id=362671 (fix applied in branch 5.6, branch 5.7 and master).

Revision history for this message
In , Piotr-mierzwinski (piotr-mierzwinski) wrote :

(In reply to Leslie Zhai from comment #61)
> Yup also worked for ArchLinux now ;-)

I use Antergos (Arch based distro) and I observed next problem. I mean that konsole is not restored in such case. One day I shut down system (calling proper option in K menu) and when next day I login konsole is not restored, wheras kwrite, kate, dolphin, okular are restored. This issue not happens when I shut down computer and run it again the same day. Additionally I tried shut down using qdbus command like this: "qdbus org.kde.ksmserver /KSMServer logout 0 2 2", but with the same result.

So I wonder if this issue is related only for Arch based distributions or this is some bug in konsole :/.

Revision history for this message
In , Martin-sandsmark (martin-sandsmark) wrote :

*** Bug 366673 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Wbauer (wbauer) wrote :

*** Bug 363876 has been marked as a duplicate of this bug. ***

Changed in qtbase-opensource-src (Ubuntu):
assignee: Timo Jyrinki (timo-jyrinki) → nobody
Changed in plasma-workspace (Ubuntu):
status: Confirmed → Fix Released
Changed in qtbase-opensource-src (Ubuntu):
status: Confirmed → Fix Released
no longer affects: qt
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.