Repository navigation
Fix ttsafe rwlock writer starvation and two big-endian test failures - #6744
Merged
hyoklee merged 11 commits intoOct 23, 2026
Merged
Conversation
glibc's default pthread rwlock prefers readers, so the spinning reader threads in tts_rwlock could starve the writers forever and ttsafe hung until the CTest timeout (seen with glibc 2.17 on POWER8 ppc64). On glibc, create the lock with PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP, matching the C11 and macOS implementations, which block new readers while a writer is waiting. Fixes HDFGroup#6743 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hyoklee
requested review from
fortnern,
glennsong09,
jhendersonHDF,
lrknox,
mattjala and
vchoi-hdfgroup
as code owners
October 16, 2026 23:16
github-actions
Bot
removed request for
fortnern,
lrknox,
mattjala and
vchoi-hdfgroup
October 16, 2026 23:17
Contributor
Review ChecklistThis PR touches the following areas. Each needs a sign-off
|
- test/API/H5_api_dataset_test.c: test_create_dataset_enum_types() passed a size_t to H5Tenum_insert() for an int-sized (and a 32-bit) enum. On big-endian systems the library read the zero high-order bytes, so every member had value 0 and the test failed with "value redefinition". Use correctly sized int and uint32_t values. - HDF5Examples/C/HL/tfiles/h5ex_image2BE.ddl: the big-endian reference listed the image8bit and palette datasets as H5T_STD_U8LE instead of H5T_STD_U8BE. The mismatch made H5_H5DUMP-h5ex_image2 compare a ~41,000-line output against the wrong reference until it timed out. Both were seen on ppc64 big-endian (RHEL 7.9). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hyoklee
changed the title
# Conflicts: # release_docs/CHANGELOG.md
hyoklee
force-pushed
the
fix-glibc-rwlock-writer-starvation
branch
from
October 22, 2026 02:38
9e82cd2 to
e8c79ce
Compare
hyoklee
added
Component - C Library
| */ | ||
| if (H5_UNLIKELY(pthread_rwlockattr_init(&attr))) | ||
| HGOTO_DONE(FAIL); | ||
| if (H5_UNLIKELY(pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP))) { |
Collaborator
There was a problem hiding this comment.
This should not be the default without some kind of opt-in mechanism as it changes the semantics and can also just lead to starving readers
Member
Author
There was a problem hiding this comment.
Agreed. I've reverted the library change (and its CHANGELOG entry) in d27bce2, so H5TS_rwlock_init() is back to default attributes. The hang is now fixed in the test instead: the verify_counting readers only yielded between read locks, so with glibc's reader-preferring lock their critical sections kept overlapping and the writers never got in. They now sleep briefly (H5_nanosleep) so the lock is regularly free for the writers.
Per review, don't change the library's rwlock to writer-preferring on glibc: that changes lock semantics for all users and can starve readers. Revert the H5TSrwlock.c change and its CHANGELOG entry. Instead, fix tts_rwlock: the verify_counting readers only called H5TS_thread_yield() between read locks, so with glibc's reader-preferring default lock their critical sections kept overlapping and the writers never got in (hang on ppc64 / glibc 2.17). Sleep briefly instead so the lock is regularly free for the writers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hyoklee
changed the title
| * readers' critical sections keep overlapping and the writers | ||
| * can starve. Sleeping briefly leaves gaps where the lock is free. | ||
| */ | ||
| H5_nanosleep(10000); |
Collaborator
There was a problem hiding this comment.
Fine for now, but the test likely needs reconsideration since this should be able to be handled better with some synchronization primitive
jhendersonHDF
approved these changes
Oct 23, 2026
Contributor
|
🎉 All checklist items are signed off — @hyoklee, this PR is ready to merge. |
github-actions
Bot
added
the
checklist-complete
github-project-automation
Bot
moved this from To be triaged
to Done
in HDF5 - TRIAGE & TRACK
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
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.
Describe your changes
This PR fixes three test failures seen on a ppc64 big-endian host (RHEL 7.9, glibc 2.17, POWER8): a reader/writer starvation hang in
ttsafe, and two big-endian bugs in tests. All changes are test-only; the library is unchanged.1. Writer starvation in
tts_rwlock(ttsafehang)On glibc,
H5TS_rwlock_tis apthread_rwlock_twith default attributes, which prefer readers. Intts_rwlock, the 12verify_countingreader threads only calledH5TS_thread_yield()between read locks, so their critical sections kept overlapping and the 4count_up_and_downwriters never got the lock.H5TEST-ttsafehung until the CTest timeout on RHEL 7.9 / glibc 2.17 / POWER8 ppc64.The readers now sleep briefly (
H5_nanosleep(10000)) instead of yielding, so the lock is regularly free for the writers. (An earlier revision of this PR made the library lock writer-preferring on glibc; that was dropped after review because it changes lock semantics and can starve readers.)2. Big-endian test fixes
test/API/H5_api_dataset_test.c:test_create_dataset_enum_types()passed&i(asize_t) toH5Tenum_insert()for anint-sized enum and a 32-bit enum. On big-endian systems the library read the zero high-order bytes, so every member had value 0 andh5_api_test_datasetfailed with "value redefinition". The test now uses correctly sizedintanduint32_tvalues.HDF5Examples/C/HL/tfiles/h5ex_image2BE.ddl: the big-endian reference listedimage8bitandpaletteasH5T_STD_U8LEinstead ofH5T_STD_U8BE. Comparing against the wrong reference madeH5_H5DUMP-h5ex_image2run until the 1200 s timeout.Testing
ttsafepassed 5 runs in a row on the affected ppc64 host (it hung before). The current test-side fix still needs to be re-verified on that host.PR6744/ppc64/rh-7.9/mpich-3.3/gcc-4.8/-sh/f/mp/nr/np=4on my.cdash.org) showed the two big-endian failures above. With the test fixes applied,h5_api_test_datasetpasses andH5_H5DUMP-h5ex_image2passes in 0.36 s.Issue ticket number (GitHub or JIRA)
Fixes #6743
Checklist before requesting a review
🤖 Generated with Claude Code