Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add option to start swayidle in locked state #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gdamjan
Copy link
Contributor

@gdamjan gdamjan commented Oct 5, 2020

add a CLI option -l to start swayidle in locked state. when used, swayidle will send itself the SIGUSR1 signal as soon as it starts and subsequently start the locker.

fixes #80

@emersion
Copy link
Member

emersion commented Oct 9, 2020

when used, swayidle will send itself the SIGUSR1 signal as soon as it starts and subsequently start the locker.

Why can't you do that manually instead?

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 9, 2020

when used, swayidle will send itself the SIGUSR1 signal as soon as it starts and subsequently start the locker.

Why can't you do that manually instead?

that didn't work for me for some reason[*], but let me check again.

[*] the problem I suspect is that swayidle doesn't really support any notification of its activated state, so it's a bit hard to know when to send the signal. and I'd like to avoid random sleep 1 pauses :)

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 9, 2020

so this one, does not work

[Service]
Type=simple
ExecStart=/usr/bin/swayidle -w
ExecStartPost=/bin/kill -SIGUSR1 $MAINPID

it seems to kill the child process even before swayidle is exec'd

OTOH, this one seems to work properly, since the swayidle process is first exec'd before systemd goes to the next step:

[Service]
Type=exec
ExecStart=/usr/bin/swayidle -w
ExecStartPost=/bin/kill -SIGUSR1 $MAINPID

I do feel that the PR would still be useful since for ex. starting from environments without systemd, some shell doing this is still racy:

swayidle &
killall -USR1 swayidle

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 9, 2020

another option would be something like -f so that swayidle forks and daemonizes after it's fully initialized. That will work for shell scripts and for systemd with Type=forking

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 11, 2020

Ok on second test, just after a boot, even with Type=exec the swayidle process is killed by the USR1 signal

❯ systemctl --user status swayidle
● swayidle.service - Idle manager for Wayland
     Loaded: loaded (/home/damjan/.config/systemd/user/swayidle.service; enabled; vendor preset: enabled)
     Active: failed (Result: signal) since Mon 2020-10-12 00:32:25 CEST; 29s ago
       Docs: man:swayidle(1)
    Process: 647 ExecStart=/usr/bin/swayidle -w (code=killed, signal=USR1)
    Process: 649 ExecStartPost=/bin/kill -SIGUSR1 $MAINPID (code=exited, status=0/SUCCESS)
   Main PID: 647 (code=killed, signal=USR1)
        CPU: 3ms

Oct 12 00:32:25 archless systemd[544]: Starting Idle manager for Wayland...
Oct 12 00:32:25 archless systemd[544]: Started Idle manager for Wayland.
Oct 12 00:32:25 archless systemd[544]: swayidle.service: Main process exited, code=killed, status=10/USR1
Oct 12 00:32:25 archless systemd[544]: swayidle.service: Failed with result 'signal'.

I guess since the system was not warm enough and files weren't cached the signal killed swayidle before it could handle it. This will always be racy.

So either swayidle should implement some activation notification, or just merge this PR.

@emersion
Copy link
Member

I'd personally prefer implementing proper startup notification, as it would be generally more useful.

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 12, 2020

I'd personally prefer implementing proper startup notification, as it would be generally more useful.

sd_notify or full fork / setsid ?

@emersion
Copy link
Member

sd_notify or full fork / setsid ?

sd_notify would not be usable without systemd, so would prefer fork, even if that comes with its own issues. See also this stale PR: swaywm/swaylock#42

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 12, 2020

#82 here's the daemonize option (-f)

I still think -l is useful by itself, even with the daemonize option.

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 12, 2020

btw, #82 doesn't fix the this issue completely. obviously still racy running an external command after swayidle -f :(.

@emersion
Copy link
Member

still racy running an external command after swayidle -f

How so?

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 12, 2020

still racy running an external command after swayidle -f

How so?

I'll comment there.

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 20, 2020

I'd like to propose merging this, since:

  1. it's useful on its own, and makes a lot of sense (to me personally, but I believe to other people too)
  2. Add optional daemonization #82 is not something that can be finished quickly, and it will need much more research and testing

@mindrunner

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Option to start locked
3 participants