Skip to content

Fix silent restart checkpoint write failures - #427

Open
ANAMASGARD wants to merge 3 commits into
PecanProject:masterfrom
ANAMASGARD:fix/checkpoint-write-failure-420
Open

ANAMASGARD wants to merge 3 commits into
PecanProject:masterfrom
ANAMASGARD:fix/checkpoint-write-failure-420

Conversation

@ANAMASGARD

Copy link
Copy Markdown
Contributor

Summary

  • What: Check the stream error state and fclose() result when writing a restart checkpoint. On failure, report the checkpoint path and exit with EXIT_CODE_FAILURE (1).
  • Motivation: Failed checkpoint writes previously returned success, allowing scripts to treat an unusable checkpoint as saved.

The stream is closed exactly once. Successful checkpoint behavior and format remain unchanged.

How was this change tested?

  • Added /dev/full regression tests with and without --quiet. These failed before the fix and pass afterward.
  • Clean build with -Wall -Werror: passed.
  • make test: all 28 unit executables passed; four smoke cases passed, with one additional case marked as a forced pass.
  • make testrun: passed, including checkpoint creation and resume coverage.
  • LLVM 19 formatting and static analysis: passed for both changed files.
  • git diff --check: passed.

No smoke output fixtures were changed.

Reproduction steps

On Linux, build with make, then run from the repository root:

repo="$PWD"
run_dir="$(mktemp -d)"
fixtures="$repo/tests/sipnet/test_restart_infrastructure"

cp "$fixtures/restart.param" "$run_dir/run.param"
cp "$fixtures/restart_segment1.clim" "$run_dir/run.clim"
cp "$fixtures/events_segment1.in" "$run_dir/events.in"

(
  cd "$run_dir" || exit 1
  rc=0
  "$repo/sipnet" \
    -i "$fixtures/restart_seg1.in" \
    --no-do-main-output --quiet \
    --restart-out /dev/full || rc=$?
  printf 'Exit code: %s\n' "$rc"
)

Previously, this returned 0 without reporting the failure. With this fix:

[ERROR  ] Failed to write restart checkpoint '/dev/full'
Exit code: 1

Related issues

Checklist

  • Related approved issue is listed above.
  • PR title includes the issue number.
  • Regression tests added.
  • Documentation updated (not applicable; no interface or format changes).
  • docs/CHANGELOG.md updated with noteworthy changes.
  • Code formatting verified with clang-format 19.

Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 16:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@Alomir Alomir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great, just a couple of minor tweaks

Comment thread src/sipnet/restart.c Resolved
…ite-failure-420

Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
@ANAMASGARD
ANAMASGARD requested a review from Alomir November 6, 2026 12:01
@ANAMASGARD

Copy link
Copy Markdown
Contributor Author

@Alomir can you please once again review , thanks

This branch has not been deployed

No deployments
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.

Checkpoint write failures silently return success

3 participants