Merge into trunk : fix-greeter-race : Code : Unity 8

Status:

Merged

Approved by:

Josh Arenson on 2016-07-20

Approved revision:

2544

Merged at revision:

2572

Proposed branch:

lp:~mterry/unity8/fix-greeter-race

Merge into:

lp:unity8

Diff against target:

31 lines (+9/-3)

1 file modified

qml/Greeter/Greeter.qml (+9/-3)

To merge this branch:

bzr merge lp:~mterry/unity8/fix-greeter-race

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Josh Arenson		2016-07-19	Approve on 2016-07-20
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-07-19
Review via email: mp+300517@code.launchpad.net

Commit Message

Fix lockscreen appearing right after unlocking a locked session on the desktop.

Description of the Change

We were having a race between handling the "please lock yourself" and the "please unlock yourself" requests from logind.

Specifically, while handling the lock request, we would start a new authentication with lightdm. This involved talking to PAM and doing some thread work and processing some events. Which meant that we could process a dbus request to unlock in the middle of the forceShow() method.

At the end of that method, we show the greeter.

So I've added a guard to make sure that at the end of forceShow, we still should be showing the greeter.

* Are there any related MPs required for this MP to build/function as expected? Please list.
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
NA

* If you changed the UI, has there been a design review?
NA

Josh Arenson (josharenson) wrote on 2016-07-19:

#

Is that an actual atomic operation or does it just make the race _nearly_ impossible?

review: Needs Information

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-07-19:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1774/rebuild

review: Needs Fixing (continuous-integration)

Michael Terry (mterry) wrote on 2016-07-20:

#

I commented in IRC: this should be atomic after selectUser() is called. It's just that some callbacks can happen during selectUser().

Josh Arenson (josharenson) wrote on 2016-07-20:

#

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass? If not, please explain why.
No, unrelated.

review: Approve

1	=== modified file 'qml/Greeter/Greeter.qml'
2	--- qml/Greeter/Greeter.qml 2016-07-07 19:52:57 +0000
3	+++ qml/Greeter/Greeter.qml 2016-07-19 19:20:56 +0000
4	@@ -76,9 +76,16 @@
5	// already shown, do it manually.
6	d.selectUser(d.currentIndex, true);
7	}
8	+
9	// Even though we may already be shown, we want to call show() for its
10	// possible side effects, like hiding indicators and such.
11	- showNow();
12	+ //
13	+ // We re-check forcedUnlock here, because selectUser above might
14	+ // process events during authentication, and a request to unlock could
15	+ // have come in in the meantime.
16	+ if (!forcedUnlock) {
17	+ showNow();
18	+ }
19	}
20
21	function notifyAppFocusRequested(appId) {
22	@@ -427,8 +434,7 @@
23	target: LightDMService.greeter
24
25	onShowGreeter: root.forceShow()
26	-
27	- onHideGreeter: d.login()
28	+ onHideGreeter: root.forcedUnlock = true
29
30	onShowMessage: {
31	// inefficient, but we only rarely deal with messages

Unity 8

Merge lp:~mterry/unity8/fix-greeter-race into lp:unity8

Commit Message

Description of the Change

Preview Diff

Subscribers