Try to complete locking screen before suspend

Review Request #109693 - Created March 25, 2013 and discarded

Information
Oliver Henshaw
kde-workspace
4.10
313123
Reviewers
plasma, solid
Here's an attempt to get the screen lock to fully activate before resume starts.


This is a problem on systems that use systemd/logind to suspend as it suspends immediately, so that there's unlikely to be time for the lock process to hide the screen before the system suspends. There shouldn't be a problem on systems that use upower, as it waits a second before suspending - there's no guarantee that the screen locks in time, but it should in practice. But there are reports of problems with upower (on ubuntu, at least) - this needs further investigation to see if there's something else going on.

It's also not clear why the lock process waits a further ten or so seconds after resume before hiding the screen.


* Delay emitting org.freedesktop.ScreenSaver.ActiveChanged and replying to org.freedesktop.ScreenSaver.Lock until the lock process is ready. We can't have the ksmserver blocking in the .Lock call as this blocks the lock process (as QApplication constructs a QSessionManager which attempts to communicate with ksmserver over XSMP) so delay replying and complete the transaction when the lock process signal its readiness.

- The qml lockscreen still blocks on powerdevil as it queries supported suspend methods using the synchronous Solid::PowerManagement API. So disable this for now. This is harmless for the plasma desktop but does affect the plasma active qml locker.

- The plasma overlay locker seems to be broken. I think this is an unrelated breakage on my test system but can't be sure. It should be fine in theory, but might have a problem if there are any power-management plasmoids on it.


Other approaches to breaking the powerdevil <-> lock process deadlock are:

* Have SuspendSession::lockScreenAndWait() call face.call(QDBus::BlockWithGui, "Lock") to return to the event loop. 
- But does this mean that the entire call-chain to this point would need to be re-entrant with respect to the event loop?
* Make powerdevil Actions into transactions that are queued and signal their completion (or failure)
- Tricky and fairly wide-ranging. And probably not suitable for 4.10.
* Have the kscreenlocker_greet lock process use asynchronous dbus calls to query powerdevil rather than the synchronous API. I'm not sure if there's already some prototype asynchronous API (for libsolid2) to cannibalise for this.
* Have ksmserver or the lock process take a delay lock -  http://www.freedesktop.org/wiki/Software/systemd/inhibit - and return from org.freedesktop.ScreenSaver.Lock as soon as the lock process is running, as we do now.
- This would solve the problem on systems that use logind to suspend, but do nothing on systems that suspend with upower. As mentioned above, it's unclear what the cause of the problem is on upower systems.


The patch series is in my clone at kde:clones/kde-workspace/oliverhenshaw/kde-workspace in the branch review/screenlocker-waitforlock and comprises:


[1/5] Represent lock state with an enum


[2/5] Emit activeChanged() only when lock process ready

* synchronously call desktopResized in the screen locker app
  (this causes the lock process windows to be created)
* signal that the lock process is ready by calling
  org.kde.screensaver.saverLockReady()
* introduce a Waiting LockState
* finish locking in lockProcessReady() when receive notification over
  dbus - start locked timer and emit locked() here rather than earlier

This is based on 62c2c398611b2e6ef6e1e38ed79bc9540fc02ec9 and the old
screensaver implementation (from 4.9 and earlier). But delaying the
lock() qt signal, and thus delaying the activeChanged() dbus signal,
until the lock prcess is ready is new behaviour.


[3/5] Delay reply from dbus calls until screen is locked

This prevents e.g. suspending before the screen is properly locked.

Based on the old screensaver implementation (from 4.9 and earlier).


[4/5] Print when lock process starts and when ready

Useful for timing slow lock process creation


[5/5] Skip blocking Solid::PowerManagement call

Solid::PowerManagement API calls powerdevil synchronously, causing
deadlock until dbus timeout.
Not enough

Issues

  • 2
  • 0
  • 1
  • 3
Description From Last Updated
Stick to the code style: for (...) { } instead of for (...) { } Lamarque Souza Lamarque Souza
You should add a comment here stating what the difference between Ready and Active are and even use better names. ... Lamarque Souza Lamarque Souza
Aaron J. Seigo
Lamarque Souza
Oliver Henshaw
Martin Flöser
Oliver Henshaw
Review request changed

Status: Discarded

Change Summary:

Superseeded by logind inhibtion support in ksldapp

Loading...