search

google / xsecurelock

X11 screen lock utility with security in mind
Apache License 2.0
876 stars 65 forks source link

Optionally sleep before mapping windows #138

Closed vincentbernat closed 2 years ago

vincentbernat commented 2 years ago

This enables children some time to initialize and prepare their content. They are started before the main loop, we sleep XSECURELOCK_SAVER_DELAY_MS, then map the windows. The saver window is mapped first, then the background window and the obscurer window. This order works fine for me and I don't get a black flash anymore, despite the saver being written in Python.

On shutdown, the order is also a bit altered to reduce the flash due to the obscurer window in white.

The multiplexer is left untouched as its parent window is not mapped.

Fix #94

vincentbernat commented 2 years ago

I have been using this patch for 6 months without any issue.

divVerent commented 2 years ago

There is one thing left to be addressed - any reason not to do this? If you prefer, I can also merge this and then make that fix as a followup commit.

vincentbernat commented 2 years ago

Sorry, I don't see any comment in the PR. Which thing do you refer to?

divVerent commented 2 years ago

See above:

Issue: if mapping the windows fails, we will now no longer exit as the X11 error handler is already overridden. This would be bad, as the screen would not be locked but XSecureLock would report a locked screen to other processes e.g. by XSS_SLEEP_LOCK_FD.

I propose moving the XSetErrorHandler call right here, after the XFlush and before the ConnectionNumber call.

vincentbernat commented 2 years ago

Sorry for the delay. I have updated the commit as you proposed. It makes sense.

BTW, I am pretty sure I don't see the comment you mention. Maybe it was made private for people inside the org? I have double checked using a private window.

divVerent commented 2 years ago

Odd - anyway, thanks - the fix looks good now. Merging.

divVerent commented 2 years ago

Thank you!

kriansa commented 2 years ago

@vincentbernat I noticed that after setting this env var, if I start xsecurelock and I have two monitors, then the primary goes first, then there's a noticeably delay (~200ms?) after the second monitor gets blank. Without this variable all of them blanks at the same time. Am I doing something wrong?

vincentbernat commented 2 years ago

I don't have this delay on my dual screen setup. I am using 500 for XSECURELOCK_SAVER_DELAY_MS. Do you set 200 in your case?

kriansa commented 2 years ago

No, I set different times just to see if there was any relation between the delay and the amount of XSECURELOCK_SAVER_DELAY_MS, but the delay is static. Anyway, let's see if someone else also reports it.