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

Feature dtcenter/METplus-Internal#21 signal handling #2336

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

georgemccabe
Copy link
Collaborator

@georgemccabe georgemccabe commented Nov 3, 2022

Expected Differences

  • Do these changes introduce new tools, command line arguments, or configuration file options? [No]

  • Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]

Pull Request Testing

  • Describe testing already performed for these changes:

I made some changes to the code and ran some tests to verify that the csignal types were handled properly. I was not able to cause a SIGILL exception, which according to the GNU docs is:

The name of this signal is derived from “illegal instruction”; it usually means your program is trying to execute garbage or a privileged instruction.

Here are the tests I performed:

  • Added int[foo]={23};delete foo; to the main function, which caused SIGABRT:

free(): invalid pointer
FATAL: Received Signal Abort. Exiting 6

  • Added int foo = 3/0; to the main function, which caused SIGFPE:

FATAL: Received Signal Floating-Point Exception. Exiting 8

  • Added abort(); to the main function, which caused SIGABRT:

FATAL: Received Signal Abort. Exiting 6

  • Ran mtd, then pressed CTRL+C to kill the process, which caused SIGINT:

FATAL: Received Signal Interrupt. Exiting 2

  • Ran mtd, then ran snuff mtd in another window to kill the process, which caused SIGTERM:

FATAL: Received Signal Terminate. Exiting 15

  • Added void (*foo)();foo(); to the main function, which caused a seg fault:

FATAL ERROR (SEGFAULT): Process 24433 got signal 11 @ local time = 2022-11-03 20:29:40Z
FATAL ERROR (SEGFAULT): Look for a core file in /d1/personal/mccabe/MET/src/tools/other/mode_time_domain
FATAL ERROR (SEGFAULT): Process command line: /home/mccabe/MET/bin/mtd -v 2 -fcst /d1/personal/mccabe/out/stage/file_lists/20170510040000_mtd_fcst_APCP.txt -obs /d1/personal/mccabe/out/stage/file_lists/20170510040000_mtd_obs_P01M_NONE.txt -config /d1/personal/mccabe/METplus/parm/met_config/MTDConfig_wrapped -outdir /d1/personal/mccabe/out/model_applications/precipitation/MTD_fcstHRRR-TLE_obsMRMS/201705100300

The core file is a file named 'core' that is generated in the current pwd where the app is running. This was the directory I was in when I called METplus to run a use case. The core file will likely not be generated by default because the ulimit needs to be set for the core file. You can run ulimit -a to see the settings. The default core file size is 0, but running ulimit -c unlimited will set the size to unlimited. Then the seg fault will generate the core file that can be used with the debugger to get to the state of the app when the seg fault occurred so you can see where it occurred.

  • Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
  • Review the code changes
  • Try running an app and try to recreate some of the signals to see if you get the correct results.
    • There is an mtd binary on seneca that can be used to test: /d1/projects/MET/MET_issues/feature_21i/mtd OR
    • From the feature branch, build/install the basic/vx_util library, then build an app
  • Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]

  • Do these changes include sufficient testing updates? [Yes]

  • Will this PR result in changes to the test suite? [No]

  • Please complete this pull request review by 11/8/2022.

Pull Request Checklist

See the METplus Workflow for details.

  • Review the source issue metadata (required labels, projects, and milestone).
  • Complete the PR definition above.
  • Ensure the PR title matches the feature or bugfix branch name.
  • Define the PR metadata, as permissions allow.
    Select: Reviewer(s)
    Select: Organization level software support Project or Repository level development cycle Project
    Select: Milestone as the version that will include these changes
  • After submitting the PR, select Development with the original issue number.
  • After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
  • Close the linked issue and delete your feature or bugfix branch from GitHub.

…ools to log information on why the application ended unsuccessfully
@georgemccabe georgemccabe added this to the MET 11.0.0 milestone Nov 3, 2022
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve of these changes.

I reviewed the code changes and they all look fine to me. I see that all GHA tests have passed and produce no diffs. I have not confirmed that the changes actually do what is intended, but see that @georgemccabe has documented sufficient testing in the text of the PR.

@georgemccabe georgemccabe merged commit 6b5567f into develop Nov 9, 2022
@georgemccabe georgemccabe deleted the feature_21i_signal_handling branch November 9, 2022 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants