Skip to content

Remove abort on infinite loop during library close - #6532

Merged
mattjala merged 8 commits into
HDFGroup:developfrom
mattjala:abort_on_no_err_display
Aug 13, 2026
Merged

Remove abort on infinite loop during library close#6532
mattjala merged 8 commits into
HDFGroup:developfrom
mattjala:abort_on_no_err_display

Conversation

@mattjala

Copy link
Copy Markdown
Contributor

When the library detects that it cannot make progress closing itself (an "infinite loop closing library"), it now aborts regardless of whether automatic error message display has been enabled via H5Eset_auto2(). Previously the abort was skipped whenever error output was turned off, so an application that had disabled error reporting would continue running in this unrecoverable state instead of terminating.

Fixes #6531

Copilot AI review requested due to automatic review settings July 16, 2026 15:04
@mattjala mattjala added the Component - C Library Core C library issues (usually in the src directory) label Jul 16, 2026
@github-project-automation github-project-automation Bot moved this to To be triaged in HDF5 - TRIAGE & TRACK Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

✅ All areas have been signed off.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts HDF5 library shutdown behavior so that when an “infinite loop closing library” condition is detected, the abort path is no longer suppressed merely because automatic error output was disabled via H5Eset_auto2().

Changes:

  • Move the abort() call outside the if (func) block so it no longer depends on error auto-display being enabled (still under #ifndef NDEBUG).
  • Document the behavior change in release_docs/CHANGELOG.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/H5.c Ensures the abort path is not contingent on whether automatic error printing is enabled.
release_docs/CHANGELOG.md Adds a changelog entry describing the updated shutdown/abort behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/H5.c Outdated
@jhendersonHDF

Copy link
Copy Markdown
Collaborator

Introduced in 339a81a, but no particular rationale given. I assume the intention is that if the user asked to disable error reporting, then they don't want any error handling path like this to be triggered. The library is still going to terminate either way, so I don't think skipping the abort() carries any danger whatsoever. It's also arguably rude for a library to ever call abort(), even if it's during library termination, since it doesn't give any chance for cleanup and can have any number of effects on downstream code.

lrknox
lrknox previously approved these changes Jul 16, 2026
@lrknox
lrknox dismissed their stale review July 16, 2026 21:12

I noticed the comments afterward and agree with at least the ones regarding the release note.

@mattjala mattjala changed the title Abort on infinite loop even when error output is disabled Remove abort on infinite loop during library close Jul 17, 2026
@github-actions
github-actions Bot requested a review from lrknox July 20, 2026 16:06
lrknox
lrknox previously approved these changes Jul 20, 2026

@lrknox lrknox 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.

CHANGELOG.md has 2.2.0 release note entries that have been removed in develop; they should also be removed here. Removing abort has been discussed, was removing it settled?

@github-project-automation github-project-automation Bot moved this from To be triaged to In progress in HDF5 - TRIAGE & TRACK Aug 6, 2026
@mattjala

mattjala commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

CHANGELOG.md has 2.2.0 release note entries that have been removed in develop; they should also be removed here. Removing abort has been discussed, was removing it settled?

Yes, we decided that removing the abort makes the most sense.

@github-actions
github-actions Bot requested a review from lrknox August 6, 2026 20:13
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mattjala
mattjala merged commit e4b6a96 into HDFGroup:develop Aug 13, 2026
129 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Done in HDF5 - TRIAGE & TRACK Aug 13, 2026
@mattjala
mattjala deleted the abort_on_no_err_display branch August 13, 2026 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component - C Library Core C library issues (usually in the src directory)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

abort occurs only if display error message is enabled

4 participants