Bug 313123 - screenlocker may not fully activate until after resume from suspend
Summary: screenlocker may not fully activate until after resume from suspend
Status: RESOLVED FIXED
Alias: None
Product: kscreensaver
Classification: Miscellaneous
Component: locker-qml (show other bugs)
Version: 4.10.0
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords:
: 316275 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-01-12 09:54 UTC by m.wege
Modified: 2014-05-30 22:40 UTC (History)
15 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.11


Attachments
extra debugging statements to diagnose locking problems (4.84 KB, patch)
2013-03-16 20:12 UTC, Oliver Henshaw
Details
fix attempt (1.02 KB, patch)
2013-03-25 22:37 UTC, Thomas Lübking
Details

Note You need to log in before you can comment on or make changes to this bug.
Description m.wege 2013-01-12 09:54:13 UTC
I could not find a component for the new screenlooker in KDE. I hope I am right, choosing Kwin. As I recall one of the ideas behind the new screensaver/screenlooker was to have full control over the screen content and prevent parts of the screen to be revealed. I am running 4.10 RC2 on Kubuntu and I have to report that this goal was not achieved yet. I frequently suspend to Ram by closing the laptop lid and when I open the lid again I first see the screen content when the laptop awakes from suspension. Shortly after the screen looker kicks in.

Reproducible: Always
Comment 1 Martin Flöser 2013-01-12 11:41:44 UTC
KWin is actually the wrong component as KWin has nothing to do with it.

What you experience is probably unfixable. The system goes to suspend before the screenlocker loaded. I wanted to move to powerdevil, but cannot find it. So unassigned KDE bugs.
Comment 2 Thomas Lübking 2013-01-12 13:50:19 UTC
It is *not* unfixable, but the correct fix is perhaps out of KDEs reach and scope (i don't know)

Basically, if i wanted to lock my screen on STR (unconditionally) i'd place a script in /usr/lib/pm-utils/sleep.d/ or if it's for certain power events by configuring acpid, see eg. https://wiki.archlinux.org/index.php/Acpid#Tips_and_tricks

I don't know whether powerdevil overrides this aspects (or even can, given the power event might not lock?) or is merely a config frontend, but dario hopefully does ;-)
Comment 3 Oliver Henshaw 2013-02-07 15:08:27 UTC
Can you reproduce this with 4.10 final?
Comment 4 Oliver Henshaw 2013-02-07 15:14:27 UTC
(I ask because it's possible that this was the result of some transparency problems, not because I think the timing of the screenlocker might have changed.)
Comment 5 m.wege 2013-02-07 15:35:22 UTC
Still happen in the 4.10 release. Just tested in on a fresh installation. It takes some time until the system goes to sleep, so actually there should be enough time to let the screen saver kick in.
Comment 6 Oliver Henshaw 2013-02-22 11:53:48 UTC
I naively expect the screen to lock promptly (once the dbus message reaches ksmserver) since it just has to throw up a black screen and the lock process starts afterwards. But I just noticed that KSldApp::establishGrab() waits up to two seconds if it doesn't grab the keyboard or mouse on the first try.

Is there a way to do better?
Comment 7 Thomas Lübking 2013-02-22 15:09:20 UTC
(In reply to comment #6)
> Is there a way to do better?

No (except for faster polling) and this seems unlikely the problem.

If a client holds a grab on either keyboard or mouse it is for another client not possible to
a) determine the grabbing client
b) release the grab

Situations where some input will be grabbed are:
- open popup menu (usually including combobox dropdowns)
- pressed mouse on some items (eg. buttons or sliders)
- drag and drop (from a Qt application)
- mouse grabbed to pick a window (xwininfo, xprop, kcolorchooser, ksnapshot)

In these situations the grab /cannot/ be established and w/o it the entire screenlocker becomes worthless - but i doubt they're the case (unless locking is involved by a popupmenu and not emitted on the closing menu or eg. the button release but the button press)

Also, where would you want to know when the other process has actually received and processed the dbus method call?
Ask "qdbus org.freedesktop.ScreenSaver /ScreenSaver GetActive"
to know whether the screenlocker is active now or try to hook onto "org.freedesktop.ScreenSaver.ActiveChanged" to know when this is the case.
Comment 8 Eric Griffith 2013-03-06 21:33:47 UTC
Thomas, since I know you saw my comments in bug #305270 why is this 'impossible' to fix? And why was Gnome able to fix it in their code? (I realize the code bases are drastically different, im just asking what are the architectural differences that allows them to fix it but not KDE?)

Why can't the implementation be re-ordered to make sure screenlocker is called and executed first before suspend?
Comment 9 Thomas Lübking 2013-03-06 21:41:38 UTC
Who said it's impossible?

Actually the screenlocker will emit a dbus signal once it's up eg. powerdevil could wait for that for a reasonable time (like up to 5 seconds) and then suspend.

What does *not* work (never, not in gnome, xfce, yourdreamde) is to effectively lock the screen (not visually but "protecting" the input) when the input *is* grabbed on such attempt (you can retry some times, hoping the other process will ungrab the input meanwhile) - that's somehow a limitation in the X11 protocol (it's not possible to steal the grab or even determine who grabs the input atm.)

W/o grabbing the input, screenlocking makes little sense since the user could just alt+tab away (once the other process releases the grab)
Comment 10 Eric Griffith 2013-03-06 21:47:23 UTC
I assumed you meant impossible when you said "No" to "Can we do better?" (Or maybe I misunderstood you)

And yes, That is a limitation of X11, its also one of the big features of Wayland-- should finally PROPERLY handle input grabbing/releasing.
Comment 11 Thomas Lübking 2013-03-06 22:04:11 UTC
No, that was about this limitation because Oliver (see comment #6) "naively expect the screen to lock promptly" - what simply cannot be guaranteed (for wait-and-try to grab input) by the screenlocker.

This cannot be done better - powerdevil has to wait until the screenlocker says that it's ready.

(The alternative was that the screenlocker hides the screen immediately and tries to obtain the lock later, but that would trap the user into wrong assumptions about the protection level. Also there's no guarantee the grab is released "somewhen" after the resume at all.)

About having the screenlocker (or even the input grab holding ksmserver) STR - i'd say that simply does not belong into those processes - that's what IPC is there for and (power managing) powerdevil would by this rely on specific sibling behavior.
Comment 12 Oliver Henshaw 2013-03-07 12:11:58 UTC
*** Bug 316275 has been marked as a duplicate of this bug. ***
Comment 13 Oliver Henshaw 2013-03-16 19:01:35 UTC
See http://mail.kde.org/pipermail/plasma-devel/2013-March/024330.html for a discussion of this and my musings over why the screenlocker can never complete before powerdevil starts suspend even though powerdevil does try to wait for the screen to be locked.

But I now think the key to this is upower vs.  logind - upower waits for a second before suspending, and this gives the screenlocker enough time to hide the screen; while logind does not (unless asked).

By my current understanding:

1. Powerdevil calls Lock() over dbus
2. ksmserver grabs the input
3. ksmserver creates the lockwinder
    PROBLEM -> the lock window doesn't hide the screen
4. ksmserver starts the lock process
    PROBLEM -> the lock process blocks on ksmserver and on powerdevil so can't do any real work now.
5a. ksmserver emits Activechanged over dbus
5b. ksmserver replies to Powerdevil over dbus
    -> ksmserver no longer blocks the lock process
6. Powerdevil continues and triggers the suspend
   -> there's at least one brief window where powerdevil doesn't block the lock process from 
continuing
7. upower/systemd-logind suspends the system
   -> I think upower waits a second before suspending the system, I don't think logind does (unless it's asked to). I'm not sure whether the lock process can run during this time or whether powerdevil is still unresponsive.
8. The system resumes
    -> at some point the lock process finishes its job and hides the window

* With suspend from upower in fedora 17 the locker is quite reliably up before the system suspends here. But maybe it would happend shortly after resume on some systems?
* With suspend from logind in fedora 18 I can reproduce this. The screen locks about fifteen seconds after resuming here. I'm not yet sure why it's fifteen seconds, yet.
Comment 14 Oliver Henshaw 2013-03-16 19:08:11 UTC
So, some questions for those who see this:

* What version of upower and systemd (if installed) do you have?
* How long does it typically take after resume for the locker to activate?
* Can you interact with the desktop after resume and before the locker activates?

I've seen reports of the last at http://lists.fedoraproject.org/pipermail/kde/2013-March/012365.html - but don't understand why it would happen: either the grab has already succeeded or the grab has failed and the screen will not lock.
Comment 15 m.wege 2013-03-16 19:14:47 UTC
(In reply to comment #14)
> * What version of upower and systemd (if installed) do you have?
UPower client version 0.9.17
UPower daemon version 0.9.17
> * How long does it typically take after resume for the locker to activate?
say about 5 seconds, sometimes more.
> * Can you interact with the desktop after resume and before the locker
> activates?
yes
Comment 16 Oliver Henshaw 2013-03-16 20:12:01 UTC
Created attachment 78118 [details]
extra debugging statements to diagnose locking problems

Here's a patch with a load of kdebug printouts at various stages of lock/suspending/resuming.

Can you build kde-workspace with it, then:
* set KDE_DEBUG_TIMESTAMP=2 in your environment for millisecond-resolution timestamps
* active 'kded', 'ksmserver' and  'kscreenlocker_greet' in kdebugdialog. (You may need to logout and activate the screenlocker after first running kdebugdialog to get them all to appear for selection)

Then collect the .xsession-errors output when (a) the Suspend Session (or button event) action is set to lock the screen, and (b) when it is set to hibernate or suspend, and the action is activated (by you or on idle).
Comment 17 m.wege 2013-03-16 20:17:07 UTC
Comment on attachment 78118 [details]
extra debugging statements to diagnose locking problems

Sorry, I am not capable of building a whole KDE environment. I could install (Kubuntu) packages though.
Comment 18 Thomas Lübking 2013-03-16 20:37:02 UTC
1. this happens here w/ powerdevil as well

2. if i 
QDBusInterface("org.freedesktop.ScreenSaver", "/ScreenSaver",
                    "org.freedesktop.ScreenSaver", QDBusConnection::sessionBus()).call(QLatin1String("Lock"));
QDBusInterface("org.freedesktop.UPower", "/org/freedesktop/UPower",
                            "org.freedesktop.UPower", QDBusConnection::systemBus()).call(QLatin1String("Suspend"));

this does not happen.

3. If i call (really through bash!)
qdbus org.freedesktop.ScreenSaver /ScreenSaver Lock; qdbus org.freedesktop.PowerManagement /org/freedesktop/PowerManagement Suspend

this does not happen either (i turned off the locking feature in powerdevil)

When the system comes up from the failed powerdevil run it takes ~ 300ms to lock the screen.

PS:
+    kDebug() << "Locking screen";
     QDBusPendingReply< void > reply = iface.Lock();
     reply.waitForFinished();
+    kDebug() << "Finished waiting";

Have you checked that this does really timeout and not QDBusPendingReply being smart like "well, we know the answer to void ..."

You could also try performing a sync dbus call (like above) instead.
I won't suspend my box for today again, poor little ac ;-)
Comment 19 Richard Llom 2013-03-21 20:13:45 UTC
(In reply to comment #14)
> * What version of upower and systemd (if installed) do you have?
UPower client version 0.9.19
UPower daemon version 0.9.19
systemd 197
+PAM +LIBWRAP -AUDIT -SELINUX -IMA +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ

> * How long does it typically take after resume for the locker to activate?
About the same time if I run it from "hand", thats about 9 (long) seconds here.

> * Can you interact with the desktop after resume and before the locker
> activates?
I can move the mouse around, but I couldn't click anything. Couldn't figure out yet if this is due to the 100% CPU upon resume or if it is really locked.

Sorry, I cannot build a KDE Workspace either. I'm on Chakra tho (if that helps).
Comment 20 Richard Llom 2013-03-22 12:54:57 UTC
(In reply to comment #19)
> > * Can you interact with the desktop after resume and before the locker
> > activates?
> I can move the mouse around, but I couldn't click anything. Couldn't figure
> out yet if this is due to the 100% CPU upon resume or if it is really locked.
I checked again, I'm pretty sure now that the screen is "locked". I had a blinking cursor and I couldn't type anything, scroll and mouse clicks didn't worked either.
However this is still annoying and a privacy issue.


While I was looking for this issue, I also found this:
bug 297068 comment#3 and follow up
is this maybe related?
Comment 21 Oliver Henshaw 2013-03-24 18:25:49 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > * What version of upower and systemd (if installed) do you have?
> UPower client version 0.9.17
> UPower daemon version 0.9.17

Odd. Can you tell me the results of "time qdbus --system org.freedesktop.UPower /org/freedesktop/UPower org.freedesktop.UPower.Suspend" (or Hibernate if Suspend isn't supported on your system) and "time qdbus org.freedesktop.ScreenSaver /ScreenSaver org.freedesktop.ScreenSaver.Lock" (do it twice and tell me the second time, so there's a warm cache). Thanks.
Comment 22 Oliver Henshaw 2013-03-24 18:34:33 UTC
(In reply to comment #19)
> (In reply to comment #14)
> > * What version of upower and systemd (if installed) do you have?
> UPower client version 0.9.19
> UPower daemon version 0.9.19
> systemd 197
> +PAM +LIBWRAP -AUDIT -SELINUX -IMA +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ

OK. I understand why this happens with systemd/logind controlling suspend. But not why it takes so long after resume to kick in.

> > * How long does it typically take after resume for the locker to activate?
> About the same time if I run it from "hand", thats about 9 (long) seconds
> here.

I'm not sure what you mean - do you mean it takes 9 seconds to lock the screen if you're just locking it and not suspending? What's the result of "time qdbus org.freedesktop.ScreenSaver /ScreenSaver org.freedesktop.ScreenSaver.Lock" in this case?

(In reply to comment #20)
> While I was looking for this issue, I also found this:
> bug 297068 comment#3 and follow up
> is this maybe related?
It's not related, I think. Well, it's partly fallout from the locker moving to ksmserver, but that's all.
Comment 23 Oliver Henshaw 2013-03-24 18:37:38 UTC
(In reply to comment #18)
> PS:
> +    kDebug() << "Locking screen";
>      QDBusPendingReply< void > reply = iface.Lock();
>      reply.waitForFinished();
> +    kDebug() << "Finished waiting";
> 
> Have you checked that this does really timeout and not QDBusPendingReply
> being smart like "well, we know the answer to void ..."
Yep. If I sprinkle sleep()s in ksldapp.cpp I do indeed see the same delay on the other end of the dbus message.
Comment 24 Richard Llom 2013-03-25 19:27:51 UTC
(In reply to comment #22)
> (In reply to comment #19)
> > (In reply to comment #14)
> > > * How long does it typically take after resume for the locker to activate?
> > About the same time if I run it from "hand", thats about 9 (long) seconds
> > here.
> I'm not sure what you mean - do you mean it takes 9 seconds to lock the
> screen if you're just locking it and not suspending?
Yes.

> What's the result of "time qdbus org.freedesktop.ScreenSaver 
> /ScreenSaver org.freedesktop.ScreenSaver.Lock" in this case?
Command is not really useful:
real    0m2.277s
user    0m0.013s
sys     0m0.020s
the input is maybe locked then, but not the screen. Until I see the screen locked it took another ~12sec in this example.

Also I noticed that the time needed for screen lock is highly depend on system/RAM usage. After a fresh start its quite fast ~3secs, but then with more apps, RAM usage etc. it increases.
Comment 25 Oliver Henshaw 2013-03-25 19:43:48 UTC
(In reply to comment #24)
> (In reply to comment #22)
> > (In reply to comment #19)
> > > (In reply to comment #14)
> > I'm not sure what you mean - do you mean it takes 9 seconds to lock the
> > screen if you're just locking it and not suspending?
> Yes.
> 
> > What's the result of "time qdbus org.freedesktop.ScreenSaver 
> > /ScreenSaver org.freedesktop.ScreenSaver.Lock" in this case?
> Command is not really useful:
> real    0m2.277s
> user    0m0.013s
> sys     0m0.020s
> the input is maybe locked then, but not the screen. Until I see the screen
> locked it took another ~12sec in this example.

That's a long time indeed. Here it returns in about 300 milliseconds and it's another second or less before the screen is actually hidden. And yes, it's expected that there's a delay between the two things - that's part of the problem.

> Also I noticed that the time needed for screen lock is highly depend on
> system/RAM usage. After a fresh start its quite fast ~3secs, but then with
> more apps, RAM usage etc. it increases.

Is your system very underpowered or very overloaded? Even three seconds is quite slow, or so it seems from here.

Did the old screensaver in 4.9 and earlier use to take this long to lock, or is this a new development?
Comment 26 Orion Poplawski 2013-03-25 19:47:26 UTC
For my under-powerd netbook, it is indeed worse with 4.10 than with 4.9.
Comment 27 Thomas Lübking 2013-03-25 21:29:43 UTC
the problem is that "showLockWindow" does about everything but showing the lockwindow (instead it's waited until the unlocker shows up, that one pot. takes to long because there's apparently an anchor recursion in the plasma button qml, but that's irrelevant in this regard)

Simply "show" is not gonna work since the unlock dialog won't make it on top of the lockwindow then, i'm looking into it.
Comment 28 Richard Llom 2013-03-25 22:34:20 UTC
(In reply to comment #25)
> > Also I noticed that the time needed for screen lock is highly depend on
> > system/RAM usage. After a fresh start its quite fast ~3secs, but then with
> > more apps, RAM usage etc. it increases.
> 
> Is your system very underpowered or very overloaded? Even three seconds is
> quite slow, or so it seems from here.
> 
I have
Intel(R) Pentium(R) Dual  CPU  T2330  @ 1.60GHz
with 1 GB RAM, 2GB Swap which both have some reserve left on testing. However, there is always heavy "rattling" on the HDD, when the screen locks load.

> Did the old screensaver in 4.9 and earlier use to take this long to lock, or
> is this a new development?
> 
Sorry, I don't know since I have only experience with the current KDE on this machine.
Comment 29 Thomas Lübking 2013-03-25 22:37:06 UTC
Created attachment 78388 [details]
fix attempt

The attached patch does:
1. show() the lockwindow
2. remove the waitForReadyRead() - i'm not sure what data the process is supposed to write, but the qml greeter here writes nothing, so the entire thing just times out for *a complete minute* (blocking the thread what made me think there would be an issue with the greeter not being raised, but the map notification just got not handled before)

The idea here is that the (protecting!) lockwindow shows up immediately and esp. before dbus returns. Whenever the lockdialog and (btw, this seems another bug: i cannot disable that through the GUI at least) the fancy show  show up does really not matter (1ms before the suspend or 1s after the resume - "who cares")

-> please test anyone.
Comment 30 Thomas Lübking 2013-03-25 22:38:52 UTC
(In reply to comment #28)
> there is always heavy "rattling" on the HDD, when the screen locks
> load.

This gets *slightly* off topic, but what filesystem do you keep /var on?
Comment 31 Richard Llom 2013-03-25 23:33:42 UTC
(In reply to comment #29)
>  fix attempt
> 
> -> please test anyone.
I would need instructions for that.

(In reply to comment #30)
> This gets *slightly* off topic, but what filesystem do you keep /var on?
It is ext4 on an extra (logical) partition - same HDD tho.
Comment 32 Thomas Lübking 2013-03-25 23:46:07 UTC
You'll need to get kde-workspace from git, apply the patch and then configure, compile & install ksmserver.
Then re-login.

On git:
http://techbase.kde.org/Development/Tutorials/Git/GitQuickStart

In konsole:
---------------
git clone git://anongit.kde.org/kde-workspace"
cd kde-workspace
patch -p1 < /path/to/patch.diff
mkdir build
cd build
ccmake ..
[press "c", then fix installation path etc. then press "g" and "q"]
cd build/ksmserver
make && sudo make install

if you never build KDE or actually anything before and don't really intend to get in touch with development and use a developer unfriendly distro where you need to install dev packages in order to get library headers, maybe wait whether eg. oliver (*cough*) can say anything about the patch =)
Comment 33 Oliver Henshaw 2013-03-26 22:08:31 UTC
(In reply to comment #29)
> Created attachment 78388 [details]
> fix attempt

On fedora 17 it works fine. On fedora 18 locking is slightly slower. And on both virtual machines spice+qxl is slower than vnc+cirrus. The black lock window appears and then a lighter grey rectangle sweeps across the top half of the screen twice before the greeter window and unlock dialog appears.

Anyway, I think this is right. The desktop can't be seen any more on resume on the fedora 18/systemd machine. Instead there's a black screen with  optional sweeping rectangles for the several seconds it takes the unlock screen to appear. This isn't pleasant, but at least we're now dealing with polish and performance issues. Which is an improvement, I guess.

> The attached patch does:
> 1. show() the lockwindow
I've just discovered 21ae28ccd77f9a354cf0855a44b47e1cc6ae8dec - reverting this also works.

There's also 1fc75fd2334c028c627f6844dfa9f641b599690e as a follow-up. I'm not sure if this was related or a fix for a separate problem. Anyway, it explains the transparent background and re-implemented paintEvent() weirdness discussed in https://git.reviewboard.kde.org/r/108643/ - I'm not sure whether this flicker on multiple monitors is still a concern?

> 2. remove the waitForReadyRead()
This is from 62c2c398611b2e6ef6e1e38ed79bc9540fc02ec9 which is only in master. This should probably be reverted, I guess.
Comment 34 Oliver Henshaw 2013-03-26 22:22:00 UTC
(In reply to comment #33)
> And on
> both virtual machines spice+qxl is slower than vnc+cirrus. The black lock
> window appears and then a lighter grey rectangle sweeps across the top half
> of the screen twice before the greeter window and unlock dialog appears.
> 
> There's also 1fc75fd2334c028c627f6844dfa9f641b599690e as a follow-up. I'm
> not sure if this was related or a fix for a separate problem. Anyway, it
> explains the transparent background and re-implemented paintEvent()
> weirdness discussed in https://git.reviewboard.kde.org/r/108643/ - I'm not
> sure whether this flicker on multiple monitors is still a concern?

So the background colour is the one that redraws, the colour in paintEvent does not. If the background colour is set to transparent it shows as black with no redrawing (with 24 bit depth) - perhaps Qt::transparent backgrounds and the paintEvent do double buffering or something and the Qt::black background doesn't due to WA_NoSystemBackground? But you probably have better insight into this stuff than me.
Comment 35 Thomas Lübking 2013-03-26 23:25:18 UTC
(In reply to comment #33)

> Anyway, I think this is right.
"Semi"
The attack on this context would be to get a bogus greeter (binary or qml) into the path so that launching the greeter will fail.

What then happens is.
1. you STR
2. black lockwindow protects your screen
3. "fine" - you go for lunch
4. i resume your machine and wait 60 seconds
5. ksdlapp notes that is completely failed to bring up the unlock dialog and to not irreversibly lock the user out removes the lock
6. profit
7. i restore the bogus unlock dialog and STR
8. you return from lunch and consider everything fine.

What really *should* happen is that 
1. powerdevil attempts to lock the screen
2. ksdlapp establishes the lock and tries to call the unlock dialog (the qml greeter)
3. once the greeter is up, it fires a dbus signal "yeeehaa - screen is locked"
4. powedevil finally goes STR because the screen is now reliably locked.

Bringing up the lockwindow instantly is an improvement to the present situation and works as long as the greeter is intact - but not beyond.

> optional sweeping rectangles for the several seconds it takes the unlock
What precisely do you mean by "sweeping rectangles"? Like "parts of the screen get exposed"?

That sounds like a known bug in some drivers for ARGB windows and no compositor in place.
The exposure events get ignored and when a drawable above in the stack gets withdrawn that area is not recovered by the window.
This was/is pretty famous with plasma tooltips (carving regions out of windows in regular sessions)

If this happens that is *possibly* an issue of the virtual machine or VNC context only.

> I've just discovered 21ae28ccd77f9a354cf0855a44b47e1cc6ae8dec - reverting
> this also works.
Yes, this introduced the code that delayed showing the lock until there's actually a greeter.

> There's also 1fc75fd2334c028c627f6844dfa9f641b599690e as a follow-up.
Aha.
Well, the idea here seems as well that the user should ideally not see the black locking window.
Because it's up with the first greeter and the second greeter window will launch considerably after the first (because it takes until Qt figures there's a recursion and gives up and because mapping windows on X11 is a matter of its own anyway) the second screen will first show the black window and than the greeter later on.
With the patch the greeter would (in a composited environment) be invisible until it receives the first paintevent (what unfortunately was never ;-)

> sure whether this flicker on multiple monitors is still a concern?
I'd say once we got security in shape we care about beauty.

However we can by no means leave any screen region exposed anyway, so my suggestion would be *then* to actually wait until the first greeter appears, then wait ~150ms then show the lockwindow and then finally emit the org.freedesktop.ScreenSaver.ActiveChanged signal and then (and not before) move to STR.

If the screen cannot be locked for whatever reason the user should not be trapped into a misassumption about the state but stay at his system, notice it won't get down, eventually get a warning dialog from ksdlapp ("cannot lock screen because blahblahblah") and fix that or log out to protect his data.

(In reply to comment #34)
> So the background colour is the one that redraws, the colour in paintEvent does not.
Yesno. No.
The "problem" here is that actually the paintEvent virtually never got called (and i doubt it gets as of today) because of the other widget attributes (than foremost Qt::WA_PaintOutsidePaintEvent).
The backgroundColor hits due to Qt::WA_NoSystemBackground and Qt::WA_PaintOnScreen
This code really seems Qt3-a-like (while WA_PaintOnScreen spares you the pointless doublebuffer overhead)

If one wanted to clean up the onde, one could just setAutofillBackground(true) and scratch the paintEvent implementation

> If the background colour is set to transparent it shows as black
... as soon as there's no compositing, thus alpha channel handling.

If you've a 32bit drawable ("window") you'll get it transparent with active compositing.
The only thing that could happen (i didn't look this up since Qt 4.6) is that setting an alpha channeled background color implicitly calls setAttribute(Qt::WA_TranslucentBackground)

> and the paintEvent do double buffering
No. Doublebuffering is explicitly disabled. It's also not required or related to top level ARGB handling.
Comment 36 Oliver Henshaw 2013-03-27 00:06:18 UTC
Just a quick note. I'll mull over the rest later.

(In reply to comment #33)
> On fedora 17 it works fine. On fedora 18 locking is slightly slower. And on
> both virtual machines spice+qxl is slower than vnc+cirrus. The black lock
> window appears and then a lighter grey rectangle sweeps across the top
> half of the screen twice before the greeter window and unlock dialog appears.
This is what I mean by "sweeping rectangles", it's not exposing the desktop. Maybe it could be described as a slow motion flicker that moves from left to right. Do you have a spice/qxl virtual machine you can play with to reproduce this, or should I make a video?
Comment 37 Thomas Lübking 2013-03-27 12:15:03 UTC
(In reply to comment #36)
> Just a quick note. I'll mull over the rest later.

> This is what I mean by "sweeping rectangles", it's not exposing the desktop.

Ok. NO idea what that would be but assume it's related to "greeter needs 3 seconds to launch" - so i'm not concerned either (we'll care about this once security and launch time are resolved)

Do you want to revert 21ae28ccd77f9a354cf0855a44b47e1cc6ae8dec for 4.10.2 (tomorrow) or should I create a RR or should we resolve this really for 4.10.3?
Comment 38 Richard Llom 2013-03-28 09:23:16 UTC
(In reply to comment #37)
> Do you want to revert 21ae28ccd77f9a354cf0855a44b47e1cc6ae8dec for 4.10.2
> (tomorrow) or should I create a RR or should we resolve this really for
> 4.10.3?
> 
If it is not to late, I would vote for 4.10.2 ...
Comment 39 Oliver Henshaw 2013-03-28 22:56:49 UTC
(In reply to comment #37)
> Do you want to revert 21ae28ccd77f9a354cf0855a44b47e1cc6ae8dec for 4.10.2
> (tomorrow) or should I create a RR or should we resolve this really for
> 4.10.3?
Yep, reverting 21ae28ccd7 is probably the best bet for now. Some evolution of https://git.reviewboard.kde.org/r/109693/ would be the best solution.

(In reply to comment #35)
> The attack on this context would be to get a bogus greeter (binary or qml)
> into the path so that launching the greeter will fail.
Perhaps. But this could only be achieved by making m_lockProcess->waitForStarted() fail, as far as I can see. And AIUI this would only happen if the process fails to start, perhaps because of the wrong permissions or because the file doesn't exist? Does this fail instantly in these cases or would it wait for the timeout?
Comment 40 Oliver Henshaw 2013-03-28 23:03:03 UTC
m.wege, could you get me the outputs of the commands below? I can explain everyone else's problems as being triggered by the use of systemd, but not yours.

(In reply to comment #21)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > * What version of upower and systemd (if installed) do you have?
> > UPower client version 0.9.17
> > UPower daemon version 0.9.17
> 
> Odd. Can you tell me the results of "time qdbus --system
> org.freedesktop.UPower /org/freedesktop/UPower
> org.freedesktop.UPower.Suspend" (or Hibernate if Suspend isn't supported on
> your system) and "time qdbus org.freedesktop.ScreenSaver /ScreenSaver
> org.freedesktop.ScreenSaver.Lock" (do it twice and tell me the second time,
> so there's a warm cache). Thanks.
Comment 41 Thomas Lübking 2013-03-28 23:49:54 UTC
(In reply to comment #39)
> Perhaps. But this could only be achieved by making
> m_lockProcess->waitForStarted() fail, as far as I can see.

Not exclusively.
If you prevent launching the call to Lock will fail. A black screen would be shown and the withdrawn immediately. The then system would go STR and be exposed on resume.

If you can make the greeter sth. like

#!/bin/sh
sleep 20; rm $0

a black screen would appear and you move STR right afterwards (looks good)
On resume, the sleep would time out, and then the "greeter" be removed. Then the "greeter" exits. The locker attempts to restart it but the greeter isn't there, doesn't launch and the lock released.

The "most important" aspect on this kind of attack is that we're talking about a trap "only" (you believe everything is fine, but it's not) - the attacker had to have had sufficient access to the system anyway to establish this in the first place.
Comment 42 Oliver Henshaw 2013-03-29 00:12:24 UTC
(In reply to comment #41)
> a black screen would appear and you move STR right afterwards (looks good)
> On resume, the sleep would time out, and then the "greeter" be removed. Then
> the "greeter" exits. The locker attempts to restart it but the greeter isn't
> there, doesn't launch and the lock released.
I think if the greeter exit(0)'s that's taken as a signal to unlock.

> The "most important" aspect on this kind of attack is that we're talking
> about a trap "only" (you believe everything is fine, but it's not) - the
> attacker had to have had sufficient access to the system anyway to establish
> this in the first place.
Yeah. If the attacker is running arbitrary code in the greeter binary you've lost in innumerable ways. I'm not sure how dangerous attacker-controlled QML could be though.
Comment 43 m.wege 2013-03-30 18:30:31 UTC
time qdbus --system org.freedesktop.UPower /org/freedesktop/UPower org.freedesktop.UPower.Suspend

real    0m28.878s
user    0m0.008s
sys     0m0.004s

time qdbus org.freedesktop.ScreenSaver /ScreenSaver org.freedesktop.ScreenSaver.Lock

real    0m0.112s
user    0m0.008s
sys     0m0.004s
Comment 44 Oliver Henshaw 2013-03-30 19:26:30 UTC
(In reply to comment #43)
> time qdbus --system org.freedesktop.UPower /org/freedesktop/UPower
> org.freedesktop.UPower.Suspend
> 
> real    0m28.878s
> user    0m0.008s
> sys     0m0.004s
> 
> time qdbus org.freedesktop.ScreenSaver /ScreenSaver
> org.freedesktop.ScreenSaver.Lock
> 
> real    0m0.112s
> user    0m0.008s
> sys     0m0.004s

Thanks. Nothing unusual there, but still this is useful information.
Comment 45 Hrvoje Senjan 2013-03-30 20:02:22 UTC
Also severely hit by the bug.
Upower 0.9.20*
Systemd 196-200 (same situation with 196, 199 and 200)

http://lists.freedesktop.org/archives/devkit-devel/2013-January/001339.html
(In reply to comment #40)
> m.wege, could you get me the outputs of the commands below? I can explain
> everyone else's problems as being triggered by the use of systemd, but not
> yours.
> 
> (In reply to comment #21)
> > (In reply to comment #15)
> > > (In reply to comment #14)
> > > > * What version of upower and systemd (if installed) do you have?
> > > UPower client version 0.9.17
> > > UPower daemon version 0.9.17
> > 
> > Odd. Can you tell me the results of "time qdbus --system
> > org.freedesktop.UPower /org/freedesktop/UPower
> > org.freedesktop.UPower.Suspend" (or Hibernate if Suspend isn't supported on
> > your system) and "time qdbus org.freedesktop.ScreenSaver /ScreenSaver
> > org.freedesktop.ScreenSaver.Lock" (do it twice and tell me the second time,
> > so there's a warm cache). Thanks.

└──╼ time qdbus --system org.freedesktop.login1 /org/freedesktop/login1 org.freedesktop.login1.Manager.Suspend true


real    0m0.092s
user    0m0.007s
sys     0m0.007s

└──╼ time qdbus --system org.freedesktop.UPower /org/freedesktop/UPower org.freedesktop.UPower.Suspend* (as noted in the beggining of the comment)
Error: org.freedesktop.UPower.GeneralError
Method is deprecated, please port to org.freedesktop.login1.Manager.Suspend

real    0m0.023s
user    0m0.013s
sys     0m0.005s

└──╼ time qdbus org.freedesktop.ScreenSaver /ScreenSaver org.freedesktop.ScreenSaver.Lock
Error: org.freedesktop.DBus.Error.NoReply
Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.

real    0m25.046s
user    0m0.008s
sys     0m0.008s
Note that the locker did kick in, but after ~one minute.

Tried the patch from Thomas (comment 29), massively improves the situation, will also post time with the patch.
Comment 46 Hrvoje Senjan 2013-03-30 20:12:09 UTC
(In reply to comment #45)
> Tried the patch from Thomas (comment 29), massively improves the situation,
> will also post time with the patch.

└──╼ time qdbus org.freedesktop.ScreenSaver /ScreenSaver org.freedesktop.ScreenSaver.Lock


real    0m0.156s
user    0m0.009s
sys     0m0.006s

Forgot to mention, all of this is with master.
Comment 47 Richard Llom 2013-04-08 15:08:45 UTC
May I ask, if there has been any changes committed to the 4.10.2 branch?

I updated my system to 4.10.2 and couldn't notice any improvement so far...
Comment 48 Thomas Lübking 2013-04-08 17:19:14 UTC
Not from here - I don't maintain the screenlocker and actually don't even know who does.
Comment 49 Hrvoje Senjan 2013-04-24 21:24:44 UTC
Somehow ad805b fixed it for me. That also made the locker appear before the actual suspend.
Comment 50 Hrvoje Senjan 2013-04-24 22:34:05 UTC
(In reply to comment #49)
> Somehow ad805b fixed it for me. That also made the locker appear before the
> actual suspend.

OK, not quite true. Some of earlier Powerdevil refactoring commits fixed it.
Comment 51 S 2013-05-07 16:30:18 UTC
@Hrvoje Senjan: Sorry for the dumb question, but what is "ad805b"? a GIT commit patch number or something?

Still broken with the new KDE 4.10.3 update on openSUSE 12.3 i586. Upon resume, it shows a LOT of scary looking console spam flying by, and then it shows the desktop for several seconds, then briefly shows the lockscreen, then a black screen with mouse pointer. After moving the mouse, the lockscreen appears and is ready for input.
Comment 52 Thomas Lübking 2013-05-07 16:39:12 UTC
(In reply to comment #51)
> @Hrvoje Senjan: Sorry for the dumb question, but what is "ad805b"? a GIT commit patch number or something?

Git commit ad805b504eb3ddec063a3e9060a6d07180232a47 by Hrvoje Senjan.
Committed on 24/04/2013 at 20:02.
Pushed by hrvojes into branch 'master'.
                                                   ^^^^^^ <- "4.11"
Comment 53 S 2013-05-07 17:31:55 UTC
(In reply to comment #52)
> (In reply to comment #51)
> > @Hrvoje Senjan: Sorry for the dumb question, but what is "ad805b"? a GIT commit patch number or something?
> 
> Git commit ad805b504eb3ddec063a3e9060a6d07180232a47 by Hrvoje Senjan.
> Committed on 24/04/2013 at 20:02.
> Pushed by hrvojes into branch 'master'.
>                                                    ^^^^^^ <- "4.11"

Got it. Thanks. So we shouldn't expect this fix to show up before KDE 4.11?
Comment 54 Thomas Lübking 2013-05-07 17:42:36 UTC
acc. to Hrvoje it's actually fixed by some other commit - and i don't know which ;-)
-> Hrvoje?
Comment 55 S 2013-05-07 17:56:22 UTC
(In reply to comment #54)
> acc. to Hrvoje it's actually fixed by some other commit - and i don't know
> which ;-)
> -> Hrvoje?

Huh, weird. I'm not seeing any improvement in 4.10.3 with the openSUSE packages.
Comment 56 Oliver Henshaw 2013-05-07 18:01:57 UTC
It's d909474bc0184505c49f2d2797bcddea980c791b to 0eff99b6db9fdfc81955e44a875c48cdd86a7da8 in master - should be appropriate for 4.10, although I'm not keen on the 0eff99b6db9fdfc81955e44a875c48cdd86a7da8 part.
Comment 57 Hrvoje Senjan 2013-05-07 19:29:43 UTC
(In reply to comment #54)
> acc. to Hrvoje it's actually fixed by some other commit - and i don't know
> which ;-)
> -> Hrvoje?
Yeah, i got carried away a bit ;-)
As this was the first commit i compiled without your patch, so only with that one i noticed issue is fixed.

But for sure it can be closed as fixed in 4.11, and probably Oliver knows can users expect something with 4.10.4/5
Comment 58 Hrvoje Senjan 2013-06-09 12:06:24 UTC
Will close it. Works with master/4.11 and doesn't look something will change on this front with 4.10 branch
Comment 59 Richard Llom 2013-06-09 20:39:48 UTC
(In reply to comment #58)
> Will close it. Works with master/4.11 and
Good to know.

> doesn't look something will change on this front with 4.10 branch
Well, would have been nice for 4.10.5 ...