-
Notifications
You must be signed in to change notification settings - Fork 602
test.pl: watchdog: Simplify, clarify, recover from failure #23752
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tonycoz
reviewed
Sep 23, 2025
aaa60ee to
d985f66
Compare
tonycoz
reviewed
Sep 23, 2025
d985f66 to
51c051d
Compare
Contributor
|
How is: meant to behave? Right now with the changes from this PR a simple test like: I get: One option here would be for all entries to the watchdog() function to clear existing watchdogs and then return immediately after if the timeout is zero With that the above produces: I tested with: |
Parenthesize the ternary conditional for clarity Change some indentation, mostly in preparation for later commits
In the thread documentation, I noticed that unsafe signals preclude killing threads with a signal. But this is the normal method for clearing a watchdog timer under threads. I doubt that people are compiling with PERL_OLD_SIGNALS these days, so I didn't check for that, but it's easy enough to check for the environment variable that does the same thing at runtime.
Prior to this commit, the code gave up immediately if it tried unsuccessfully to use a watchdog thread. But the alarm method is still available. Drop down to try it.
The next commit changes things, so that some people may find that after that, the current name would be misleading or confusing.
The fallback method of setting a watchdog timer is to use alarm(). Prior to this commit, the code presumed that if it wasn't one of the other possibilities, it must be this one. This is a valid assumption currently, but future commits will change that. Prepare for them with this commit.
Only one type of watchdog timer supposedly can exist. But I can see the possibility that a temporary glitch in the system caused a fallback type to be used. This commit simply makes sure that when cancelling, anything out there gets cancelled, instead of assuming there's just one possibility.
This enhances the code to work on cases like: watchdog(5); sleep 4; watchdog(10); sleep 8; Prior to this commit, you had to explicitly cancel an existing watchdog before setting a new one. As a result of this commit, all possible wathdog variables get undef'd early. This makes later undef calls redundant, so they are removed. Suggested by Tony Cook
This commit removes the existing END blocks that vary depending on the type of timer being used, and replaces them with a single END block that always exists, and simply calls watchdog(0). This already cancels whatever watchdogs are in place, doing nothing if none are. Spotted by Tony Cook Even though a test file should clear every timer before exit, this makes sure it happens.
51c051d to
9bd4628
Compare
Contributor
Author
|
fixed |
tonycoz
approved these changes
Oct 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@tonycoz noticed a simplification this makes.
If something goes wrong on WIndows and VMS, previously it gave up; now it tries an
alarm.It now doesn't try to use the thread method unless safe signals are in effect.
Some comments are clarified