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

move stack dump handler #4780

Merged
merged 1 commit into from
Apr 28, 2024
Merged

Conversation

tych0
Copy link
Member

@tych0 tych0 commented Apr 27, 2024

We set this up in main, but our test suite never calls qtile's main method, so my attempts at logging why tests were hanging would never succeed.

Instead, let's set this up in async_loop() where we set up the other signal handling, which is called by the test suite and hopefully as a result will yield stack traces when our test suite hangs.

Fixes #4778

We set this up in main, but our test suite never calls qtile's main method,
so my attempts at logging why tests were hanging would never succeed.

Instead, let's set this up in async_loop() where we set up the other signal
handling, which is called by the test suite and hopefully as a result will
yield stack traces when our test suite hangs.

Fixes qtile#4778

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
@elParaguayo
Copy link
Member

Thanks. This also seems to fix the weird issue I had with errors appearing in qtile shell.

@elParaguayo elParaguayo merged commit e6c8d6d into qtile:master Apr 28, 2024
21 checks passed
@tych0
Copy link
Member Author

tych0 commented Apr 28, 2024

This also seems to fix the weird issue I had with errors appearing in qtile shell.

oh, duh. because when you resize your terminal, that sends a SIGWINCH which we were also using to dump stacks. I wonder: should we use SIGUSR2 instead? We were previously using it for reloading, but I bet that nobody was actually using it for reloading (probably everyone just picked SIGUSR1), so I don't think it would break anything. And it wouldn't have any strange unintended consequences like SIGWINCH does.

@elParaguayo
Copy link
Member

SIGUSR2 would make more sense. There's no real reason for using it to reload the config as that can be triggered via IPC anyway.

@tych0
Copy link
Member Author

tych0 commented Apr 28, 2024

Ok, I will send a patch if you don't beat me to it :)

@elParaguayo
Copy link
Member

All yours, I'm working on something else!

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

Successfully merging this pull request may close these issues.

flaky tests do not report anything interesting
2 participants