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
Qt
New
Undecided
Unassigned
plasma-workspace (Ubuntu)
Confirmed
Medium
Unassigned
qtbase-opensource-src (Ubuntu)
Confirmed
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.

26 comments hidden view all 147 comments
Revision history for this message
Laurent Bonnaud (laurent-bonnaud) wrote :
27 comments hidden view all 147 comments
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)

Changed in kdebase-workspace:
importance: Medium → Low
Changed in kdebase-workspace:
status: Unknown → Fix Released
Changed in kwin (Ubuntu):
status: New → Confirmed
Changed in kwin (Ubuntu):
importance: Undecided → Critical
importance: Critical → Medium
tags: added: wily
summary: - kwin does not remember on which desktop to open windows on session start
+ KDE5/Qt5 does not support session restoration
description: updated
affects: kwin (Ubuntu) → plasma-workspace (Ubuntu)
Changed in qtbase-opensource-src (Ubuntu):
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Changed in kdebase-workspace:
importance: Low → Unknown
status: Fix Released → Unknown
Changed in kdebase-workspace:
importance: Unknown → Medium
Changed in kdebase-workspace:
status: Unknown → Won't Fix
Changed in kdebase-workspace:
status: Won't Fix → Confirmed
Changed in qtbase-opensource-src (Ubuntu):
status: New → Confirmed
Changed in qtbase-opensource-src (Ubuntu):
importance: Undecided → Medium
67 comments hidden view all 147 comments
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
Displaying first 40 and last 40 comments. View all 147 comments or add a comment.
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.