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

DWM crashes if I try to first enable fullscreen and then turn off fullscreen on a terminal (Alacritty) scratchpad (only on one monitor) #76

Open
isak102 opened this issue Dec 12, 2023 · 2 comments

Comments

@isak102
Copy link

isak102 commented Dec 12, 2023

I am using your dwm-fullscreen-compilation patch in combination with renamedscratchpad. They both work great in general but I have encountered one weird bug, DWM will crash if I first make a terminal scratchpad fullscreen and then try to turn off fullscreen. This only happens on one monitor (my main monitor) but not on my second monitor which is to the left of my main monitor. Another weird thing is that this only happens with my Alacritty scratchpad, not any other scratchpad. I have Spotify, Qalculate and Pavucontrol as scratchpads aswell and DWM doesn't crash if I do the same thing. However the scratchpads wont return to their original positions, their sizes will be distorted (this doesn't happen on my second monitor). This is my full dwm.c: https://gist.github.com/isak102/0a0919cfab0a8924def4df282c2e623d

I used git-bisect to find the commit that introduced the bug and it came down to this commit: https://gist.github.com/isak102/85cd77f9e27abfd1d93e7f8b0fc8b999

edit: I just realised its all floating terminal windows, not only scratchpads

@isak102
Copy link
Author

isak102 commented Dec 12, 2023

I did some debugging and it was line 2859 in my provided dwm.c that crashes the program. This was because c->w was set to a negative value on line 2859 if a floating terminal was on my right screen. I changed the line to c->w = c->oldw; and now it doesn't crash anymore. I am not 100% sure why the MIN() check is needed but this seems to work fine for me. I will leave the issue open though as this might affect others too

@bakkeby
Copy link
Owner

bakkeby commented Dec 12, 2023

Thanks for reporting. I have never seen or heard of this issue before.

I'll have to dig around a bit wrt these calculations.

My gut impression of what is going on here is that the window is resized to go into fullscreen, at which point c->oldx is set to the original floating x position.

Then after the window has done into fullscreen the application sends a ConfigureRequest, at which point c->oldx will be overwritten with the c->x again (still fine) and because dwm treats configure requests as being within (as in relative to) the monitor it adds m->mx to the given position.

Now it may be that a second ConfigureRequest comes in, at which point c->oldx may be overwritten with a value that is larger than the size of the monitor.

When exciting fullscreen it tries to first set c->x to the maximum, which will be oldx, then it tries to cap the width based on the size of the monitor and position of the client. Since oldx is greater than the width of the monitor it calculates a negative size which makes X11 crash (because it has like no sanity checks for some reason).

Alacritty sends a lot of configure requests like that.

Just setting oldw and oldh should be fine for the general case. What the calculation is trying to do is to address an edge case.

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

2 participants