Sync with main repo #72
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The size of the surface used to draw the indicator depends on the extents of the text being drawn on and under the indicator. This commit refactors the `render_frame` function so that the surface size is computed before drawing; before, `render_frame` drew onto a buffer, estimated the size the buffer should have had, and recursively called itself to try again with the estimated size, if necessary. This was done because Cairo's methods to estimate font and text size require that a cairo_t context object and an associated cairo_surface already have been set up. Since the surface size depends on the text size, the natural way to use Cairo would have a circular dependency. In order to compute sizes _before_ the buffer is created, this commit adds a 1x1 surface and a matching `test_cairo` context which is set to the same font and drawing parameters as the buffer that will be created. Font/text extent measurements should give the same results as for the final buffer.
We were ignoring the locked event. Wait for it instead.
This implements a readiness notification mechanism which works on both systemd and s6. References: swaywm#42 References: swaywm#275
Superseded by ext-session-lock-v1
The wl_buffers for the background surface only need to be updated when the output dimensions change. Using the fixed pool of two buffers to cache these buffers does not help, since if a new buffer is needed, it will have a different size than whatever buffers were cached. Furthermore, because the pool has fixed size, it is possible to run out of buffers if configure events arrive faster than pool buffers are marked not busy, which can lead to protocol errors when the background surface is committed after acknowledging a new size, but without attaching a buffer that matches that size.
This change has the additional benefit of ensuring that the position of the highlight only changes in reaction to a letter key or backspace being pressed, and not when the compositor sends a new configure event or the output needs to be redrawn for some other reason.
Allow typing new input while the previous one is validating. Can be
tested with:
ninja -C build && sway -c sway.conf
Where sway.config is:
exec ./build/swaylock
Fixes: swaywm#241
This commit establishes separate state machines for auth state (whether the password submitted is being verified or is wrong) and input state (typing indicators and clear message -- things relevant to the state of the password being typed in, before it is submitted.) This makes it possible to display the auth state while updating the input state (for example, show that the previously submitted password is 'verifying' or 'wrong' while typing another.) The two state machines interact only when submitting a password. There is some interference with the rendering code -- a 'cleared' message from the input state machine supersedes verifying/wrong messages from the auth state machine; although since the 'clear' state has a shorter timeout than the auth 'invalid' state, this is unlikely to hide the 'wrong' message.
Just send a singular newline like s6 expects. systemd doesn't support spawning a process with an FD to send readiness notifications to, instead it provides a socket name. IOW, this cannot be used directly with systemd after all. Closes: swaywm#312
If signal() is used to set a signal handler, only the first signal received will use the handler; any later signals use the default behavior (making swaylock terminate immediately, without unlocking). Using sigaction() ensures that the handler will be used every time.
If a large number of signals is sent, it is possible to fill the buffer for sigusr_fds[1] before the main loop has a chance to read from it; then the signal handler do_sigusr() will block on write.
If mlock() fails with errno EAGAIN, it should be retried up to five times, which is tracked in the retries variable. However, the `return false` statement after the switch case makes the function return false after the first failed mlock() call. Remove this statement to actually retry up to five times. Signed-off-by: Max Kunzelmann <maxdev@posteo.de>
The README for `swayidle` has a convenient link to the manpage, following suit here.
When setting an image with `--image <output>:<path>`, the image used to fail to apply if the relevant output appears some time after swaylock executes. Co-authored-by: Alexander Bakker <ab@alexbakker.me>
Calling wl_display_roundtrip() from an event handler is a bad practice in general because it nests multiple dispatches. We don't need to block here anyways.
On error, print a message and exit.
this makes sure the eventloop is initialized before any event dispatching. fixes occasional segfaults I observed on resume.
This ensures that the parent properly errors only if the password cannot be read.
PEG and other image formats may include an EXIF orientation tag which indicates in what orientation (rotation and mirroring) the image should be displayed. libgdk-pixbuf does not correct for this when loading an image, but provides a function to apply the transform after the fact, which this commit uses. Pulled from swaywm/swaybg#68
The few places that required a surface commit relied on render_frame and render_frame_background to form it form them after they had set up frame callback events. This would fail if render_frame ran out of buffers and exited early, as the caller would still wait indefinitely on the frame callback. swaylock would quite consistently run out of buffers when rendering immediately after a configure event, such as when the keypress causing render also caused outputs to be enabled from idle. Restructure the render and commit handling slightly so that the whole frame callback and commit setup is handled by the render code, which now has a single render entrypoint. This both avoids stalls from lacking commits, but also fixes the case the configure path to respect frame callbacks so we do not run out of buffers in the first place.
The child process handling PAM authentication can have multiple requests queued up. As the first success will unlock the screen, there is no point in processing anything afterwards.
It is better to have swaylock crash immediately and the compositor show a red screen than to operate in a degraded state where passwords cannot be checked and the screen cannot be unlocked.
This can happen if swaylock is mistakenly run inside a sandbox without /etc/passwd
This avoids segfaulting when a frame is rendered before a keymap has been received, which happens consistently on some systems when swaylock is started after starting the suspend process
On my machine EOF doesn't trigger POLLHUP (not sure why), instead
read() returns zero. It just prints the following message in a
loop:
[comm.c:101] Failed to read pw result: Success
Better handle this case in read_comm_reply() by making a difference
between a failed read() and a failed authentication.
swaylock was performing some read() calls with a >1 size and didn't handle partial reads. Fix this by introducing two helpers, read_full() and write_full(), to read/write a complete buffer. read_comm_request() needs to make the difference between EOF at the start of a message vs. in the middle, so needs to return a tri-state.
This reverts commit 285d029. This got accidentally pushed to master.
This averts the risk that one can queue up many password submissions by pressing enter repeatedly (or with key repeat, holding enter), much more quickly than PAM will process them. It may also improve usability by reducing unrevealed system state: now that at most one password can be queued, the "Verifying" message is always shown when a password is being validated. However, this commit does introduce a minor UI issue: if one types an invalid second password just slightly faster than PAM validation completes, it may fail to submit, and the signs of this may be hard to notice (state transition timing, presence of keypress indicator, and (if enabled) attempt count).
Other Sway projects have switched to this approach.
Let's not require the C compiler to support specific warnings, especially if half the flags are used to turn off the warnings anyways.
read_comm_request() just copies bytes around. handle_conversation() relies on the fact that the password buffer is NULL-terminated. Make sure that's the case.
If it's not, we may be reading before the buffer bounds later. (The password buffer size is aligned to the page size.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here be dragons: there may be regressions