-
-
Notifications
You must be signed in to change notification settings - Fork 550
Async refix #1847
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
base: main
Are you sure you want to change the base?
Async refix #1847
Conversation
|
Please disclose the use of AI in this contribution. |
|
AI did not generate any code in this PR. I used AI to double check syntax and have concepts I am still learning explained, but I wrote and implemented all code myself by hand. I also had AI check to ensure that my PR explanation was concise. |
|
I saw |
|
I originally wrote crt_getsignal as a public function but decided to surface the atomic flags I set instead, but wrote the code with CRT_getsignal() before changing it, and forgot to refactor that file. the latest PR should not have any reference to CRT_getsignal(). I used AI to try to ensure I was clearly explaining the code I changed. AI was used to understand concepts and check code but was expressly prohibited from generating any code as I am using this contribution to learn C and having something spoon feed it won't let me learn anything. |
I am not trying to be argumentative, just trying to clarify: I copy pasted code snippets as I wrote them into chatgpt/gemini to make sure I was using the right syntax, and to ensure understanding of the changes I made, but no tool generated the code I put in the PR. I am happy to add AI use to the PR, I wasn't sure what level of AI involvement required acknowledgement. |
|
Firstly, I apologise for the wrong accusation of using AI. Second, I'm not sure if this is the right way to the problem. If a user-triggered signal is going to terminate htop anyway, perhaps we don't need a |
|
All good. I used AI to write the PR because I am not sure how much to put in about what I did. The return is to allow the graceful shutdown mechanism already in place to process. the signal handler has double tap protection, so if the user signals again it exits immediately but this allows the existing shutdown path that includes CRT_done() and other clean up to execute with a delay that shouldn't be noticeable to end users. Since this is a graceful shutdown handling the clean up in the handler itself isn't signal safe, using ncurses and a other stdlibc stuff so I just set the flag, let the program do the graceful shutdown then print to std err out that it was an interrupt signal not a normal exit. |
Part of #1570: Graceful Signal Handling
This PR implements a graceful shutdown path for non-fatal termination signals, ensuring the terminal state is restored before the process exits.
Key Improvements:
Graceful Lifecycle: Transitions from immediate termination to a flag-based loop break, allowing the program to run its standard cleanup routines (Platform_done, CRT_done).
Double-Tap Safety: If a second termination signal is received during the shutdown phase, the handler invokes _exit() to ensure an immediate emergency stop.
Fatal Signal Integrity: Does not modify fatal signal handling (e.g., SIGSEGV); fatal handlers still take precedence and behave as they did previously.
POSIX Compliance: Returns a standard 128 + signal exit code.
Technical Implementation:
Flagging (CRT.c): The signal handler captures the signal number and sets a volatile sig_atomic_t terminate_requested flag.
Loop Interruption (ScreenManager.c): The main TUI loop checks the flag and breaks immediately upon signal receipt.
Shutdown Path (CommandLine.c): Once the loop exits and cleanup is complete, the program checks if termination was user-requested to print a status message to stderr and exit with the appropriate status.