Release ROS3 AWS event loop group and host resolver on H5close()

[hyoklee]

Describe your changes

The ROS3 VFD's AWS event loop group and host resolver were released only in an atexit() handler on non-Windows platforms. Their threads are "managed" aws-c-common threads, so another aws-c-* user in the same process that joins all managed threads during its own shutdown waits forever for them. For example, Aws::ShutdownAPI() in the AWS C++ SDK does this, and OPeNDAP BES hangs when it unloads its modules after using the ROS3 VFD.

• On non-Windows platforms, H5FD__s3comms_term() (reached from H5close()) now releases the event loop group and host resolver, so their threads exit.
• aws_s3_library_clean_up() stays in the atexit() handler. It also joins every managed thread in the process, including threads owned by other aws-c-* users, so calling it from H5close() could block on them.
• aws_s3_library_init() and the atexit() registration now happen only once, so the ROS3 VFD can be used again after H5close().
• Windows keeps the behavior from Fix deadlock in ROS3 VFD on Windows #6654: no atexit() handler is registered, and H5close() performs the full cleanup, guarded by RtlDllShutdownInProgress().

The same change passed the ROS3 VFD CI on Ubuntu, macOS and Windows (Debug and Release) in my fork, including the ros3, s3comms, h5ls and h5stat S3 tests: https://github.com/hyoklee/hdf5/actions/runs/37722760418

Issue ticket number (GitHub or JIRA)

None.

Checklist before requesting a review

• My code conforms to the guidelines in CONTRIBUTING.md
• I made an entry in release_docs/CHANGELOG.md (bug fixes, new features)
• I added a test (bug fixes, new features)

🤖 Generated with Claude Code

[github-actions]

Review Checklist

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

• src — @vchoi-hdfgroup
• release_docs — @lrknox

[jhendersonHDF]

The VFD should not be left in a half-initialized state like this between close and re-initialization of HDF5, as we cannot predict what changes to aws-c-s3 this will have on the eventual full cleanup process. All cleanup should be performed all at once, ideally during H5close() but since calling that isn't a requirement currently it has to be done during an atexit() handler. This is exactly why the Windows deadlock had to be worked around in an awkward fashion and other VFDs have similar problems. I consider this an inherent limitation of HDF5 currently and it should just be noted not to use the ROS3 VFD in this way until we can make H5close() an application requirement.

[hyoklee]

Thanks for the review, @jhendersonHDF. I understand the preference for a single cleanup point, but I think the risk is somewhat overstated for this change:

• The split state is the normal aws-c lifecycle. Initializing aws-c-s3 once per process and creating/releasing event loop groups and host resolvers independently is the standard usage pattern (it's how aws-c-s3's own tests and the C++ SDK operate). After H5close(), the library is initialized but holds no HDF5-owned event loop group or resolver, which is not an untested state.
• Cleanup ordering is unchanged. It is still: release host resolver + event loop group, then aws_s3_library_clean_up(). The only difference is that the first step can now happen earlier, during an explicit H5close(). If the application never calls H5close(), the atexit() path is exactly the same as before, and when HDF5's own atexit() termination later reaches H5FD__s3comms_term(), the release is a no-op.
• Windows is untouched. The Windows path still does the full cleanup in H5FD__s3comms_term(), so the loader-lock workaround is not affected.
• Documenting the limitation doesn't help the motivating case. The OPeNDAP BES hang happens because another aws-c-* user (Aws::ShutdownAPI()) joins all managed threads before process exit, so no atexit() handler can help. Without releasing the threads in H5close(), those applications have no workaround other than not using ROS3.

That said, I agree there are edge cases worth handling, e.g. another aws-c-* user calling aws_s3_library_clean_up() while HDF5 still believes the library is initialized. Which of these would you prefer to pursue?

1. Opt-in release: keep the current default behavior and release the event loop group/host resolver in H5close() only when requested (e.g. an environment variable or a ROS3 FAPL property).
2. Harden the current approach: keep the release in H5close() but avoid relying on a cached "library initialized" flag on re-initialization, so a cleanup by another aws-c-* user can't leave ROS3 in a bad state.
3. Document only: drop the code change and document the limitation, with a follow-up toward making H5close() an application requirement.

I'm happy to rework the PR in whichever direction you think is best.

[hyoklee]

Correction to my previous comment: after checking the source, aws-c-s3's tests and aws-crt-cpp's ApiHandle follow the same ordering (library init → event loop group/resolver → release → aws_s3_library_clean_up()), but they don't demonstrate keeping the library initialized after releasing the event loop group and later creating a new one, which is what this PR does across H5close()/re-init. Also, aws_s3_library_init()/aws_s3_library_clean_up() use a single non-refcounted flag, and ApiHandle::~ApiHandle() (run by Aws::ShutdownAPI()) calls aws_s3_library_clean_up(). So the edge case I mentioned is concrete: after the SDK shuts down, HDF5 would skip aws_s3_library_init() on re-init. Option 2 would need to address that directly.

[jhendersonHDF]

The split state is the normal aws-c lifecycle. Initializing aws-c-s3 once per process and creating/releasing event loop groups and host resolvers independently is the standard usage pattern (it's how aws-c-s3's own tests and the C++ SDK operate). After H5close(), the library is initialized but holds no HDF5-owned event loop group or resolver, which is not an untested state.

I expect the normal lifecycle for aws-c-s3 applications is different than HDF5 applications. When H5close() is called, all state associated with HDF5 should be freed. The only reason we can't do this currently is because calling H5close() isn't a requirement, so resource cleanup has to be done in atexit() handler. The only way to cleanly handle this currently is to make calling H5close() a requirement.

Cleanup ordering is unchanged. It is still: release host resolver + event loop group, then aws_s3_library_clean_up(). The only difference is that the first step can now happen earlier, during an explicit H5close(). If the application never calls H5close(), the atexit() path is exactly the same as before, and when HDF5's own atexit() termination later reaches H5FD__s3comms_term(), the release is a no-op.

Sure, also no problem here in general, other than that aws_s3_library_clean_up() should also be called, but can't currently for various reasons and therefore leaves state open across HDF5 close and open calls.

Windows is untouched. The Windows path still does the full cleanup in H5FD__s3comms_term(), so the loader-lock workaround is not affected.

Sure, no problem here.

Documenting the limitation doesn't help the motivating case. The OPeNDAP BES hang happens because another aws-c-* user (Aws::ShutdownAPI()) joins all managed threads before process exit, so no atexit() handler can help. Without releasing the threads in H5close(), those applications have no workaround other than not using ROS3.

Yes, this is a known (currently undocumented) limitation because HDF5 doesn't require calling H5close(). My main point from my first comment was that it is likely we will continue to be chasing cases like this as aws-c-s3 evolves unless we release all state associated with it on H5close(). These changes are a quick solution for a specific case.

Opt-in release: keep the current default behavior and release the event loop group/host resolver in H5close() only when requested (e.g. an environment variable or a ROS3 FAPL property).

I could see this being done as a workaround for the time being; I would prefer an environment variable approach, only because I expect it to be fixed in the future with an H5close() requirement. Going with an environment variable approach means we won't have to deprecate a ROS3 FAPL-setting API function.

Harden the current approach: keep the release in H5close() but avoid relying on a cached "library initialized" flag on re-initialization, so a cleanup by another aws-c-* user can't leave ROS3 in a bad state.

Since the variable is tracking our own internal state of whether or not we initialized AWS

Document only: drop the code change and document the limitation, with a follow-up toward making H5close() an application requirement.

This is the solution I'm most heavily leaning toward, but if OPeNDAP BES is a big enough motivating case, I could see the workaround to release as much state as possible being fine.