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

Sending SIGTERM doesn't instantly logout dwm #295

Closed
UtkarshVerma opened this issue Aug 29, 2022 · 8 comments
Closed

Sending SIGTERM doesn't instantly logout dwm #295

UtkarshVerma opened this issue Aug 29, 2022 · 8 comments

Comments

@UtkarshVerma
Copy link
Contributor

UtkarshVerma commented Aug 29, 2022

I use rofi to show me power options, hence I end up using pkill -TERM dwm to logout. However, what I've noticed is that dwm doesn't quit instantly. Instead, I have send an X event (switching workspace etc) and then it logs out.

Fixing this would be a huge quality of usage improvement, especially for me.

This issue has also been discussed in #276 .

@UtkarshVerma
Copy link
Contributor Author

I tested out a patch from @nrk and seems to work as expected. The XNextEvent() call is blocking and polling it seems to make sense as well.

@bakkeby
Copy link
Owner

bakkeby commented Aug 29, 2022

I managed to reproduce the issue in the end.

I merged in the changes referred to in #276 (comment), but I specifically associated this with the restartsig patch. I also tried the IPC patch and the issue was not present there.

@UtkarshVerma
Copy link
Contributor Author

Yeah, because the IPC patch is already relying on polling for getting the next X event. Would it not be better if we simply removed the blocking XNextEvent() call from the loop for all cases (with(out) the IPC and restartsig patches?

@bakkeby
Copy link
Owner

bakkeby commented Aug 30, 2022

No, because that is not the way it is in upstream dwm.

This is a bit of an extreme edge case where you shut down without pressing any buttons. If you don't have the restartsig patch then any kill signal will abruptly terminate the dwm process.

@UtkarshVerma
Copy link
Contributor Author

Yeah, that makes sense. Anyways everything's working fine so I guess this issue can be closed. Just a minor nitpick:
https://github.com/bakkeby/dwm-flexipatch/blob/master/dwm.c#L3211

We don't actually need the if (!running) condition as it is redundant.

@bakkeby
Copy link
Owner

bakkeby commented Aug 30, 2022

I suppose you may be right. Did you test it without that if condition?

I was thinking that there might be an edge case there where we have pending events that we'd want to skip when exiting.

@UtkarshVerma
Copy link
Contributor Author

UtkarshVerma commented Aug 30, 2022

I did test it by logging out thrice and it worked as expected. I think it's better to remove the condition in upstream for now and I'll let you know if we face any issues in the future.

@N-R-K
Copy link

N-R-K commented Aug 30, 2022

We don't actually need the if (!running) condition as it is redundant.

It's not exactly redundant in case XPending returns true; which should only happen really early on.

  1. XPending returns true - pending is set.
  2. A signal arrives - running is unset.

In this case, with the check the loop will break out early. Without the check it will go on to process one more event and then the loop will exit on next iteration due to while (running) being false.

I think it's best to keep the check, though I cannot think of any realistic scenarios where it will have a noticeable impact (processing one more event shouldn't be that big of a deal). Overall, don't have any strong opinion or way or another.

NextAlone pushed a commit to NextAlone/dwm-flexipatch that referenced this issue Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants