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

Set DXPL in API context for native VOL attribute I/O calls #4228

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

jhendersonHDF
Copy link
Collaborator

No description provided.

@jhendersonHDF jhendersonHDF added Merge - To 1.14 Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Component - C Library Core C library issues (usually in the src directory) Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub labels Mar 24, 2024
@jhendersonHDF
Copy link
Collaborator Author

jhendersonHDF commented Mar 24, 2024

This fixes testing of the LOG VOL. After #4113, the library started needing a valid DXPL to be set in the API context at all times when H5T__path_find_real is called and an API context was pushed. Previously, attribute writes and reads in the native VOL weren't setting that.

For future consideration around similar problems that may occur, in the LOG VOL's tests, there is a complex interaction going on in the LOG VOL that's something like the following:

At some point an H5Awrite call is made where the LOG VOL determines it should flush file metadata that it's been buffering up internally -> The H5Awrite call pushes an API context and heads down to the LOG VOL -> while flushing this metadata, the LOG VOL makes a few dataset optional calls down to the native VOL corresponding to H5Dget_offset for internal use, passing a new DXPL ID that it creates internally for each call -> each time those optional calls are made, they call H5CX_set_dxpl and overwrite the DXPL ID set in the API context -> we finally reach the part of H5Awrite that involves the native VOL, but H5T__path_find_real fails because it tries to get the property list for the API context's DXPL ID, which the LOG VOL already closed between the H5Dget_offset calls and getting down to the native VOL.

We probably need to make sure to call H5CX_set_dxpl anywhere where a DXPL ID is passed in to a native VOL callback function.

@qkoziol
Copy link
Contributor

qkoziol commented Mar 24, 2024

I don't have a chance to look at this right now, but it strikes me that the FUNC_ENTER_REENTER fix I've got in the work may address it.

@jhendersonHDF
Copy link
Collaborator Author

I don't have a chance to look at this right now, but it strikes me that the FUNC_ENTER_REENTER fix I've got in the work may address it.

Sure, I think a lot of this needs to be refactored to get a better handle on what's interacting with the API context and when, but for now this just matches what we do in the dataset I/O calls and seems like it was just missed.

@lrknox lrknox merged commit da60238 into HDFGroup:develop Mar 25, 2024
51 checks passed
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Mar 25, 2024
lrknox added a commit that referenced this pull request Mar 25, 2024
* Call memset before stat calls (#4202)

The buffers passed to stat-like calls are only partially filled in by
the call, leaving ununitialized memory areas when the stat buffers are
created on the stack.

This change memsets the buffers to 0 before the stat calls, quieting
the -fsanitze=memory complaints.

* Remove unused CMake configuration checks (#4199)

* Update link to Chunking in HDF5 page (#4203)

* Fix H5Pset_efile_prefix documentation error (#4206)

Fixes GH issue #1759

* Suggested header footer for NEWSLETTER (#4204)

* Suggested header footer for NEWSLETTER

* Updates

* Add NEWSLETTER.txt to h5vers script

* Fix grammar in README.md release template (#4201)

* Add back snapshot names (#4198)

* Use tar.gz extension for ABI reports (#4205)

* Fix issue with Subfiling VFD and multiple opens of same file (#4194)

* Fix issue with Subfiling VFD and multiple opens of same file

* Update H5_subfile_fid_to_context to return error value instead of ID

* Add helper routine to initialize open file mapping

* Reverts AC_SYS_LARGEFILE change (#4213)

We previously replaced local macros with AC_SYS_LARGEFILE, which is
unfortunately buggy on some systems and does not correctly set the
necessary defines, despite successfully detecting them.

This restores the previous macro hacks to acsite.m4

* Propagate group creation properties to intermediate groups (#4139)

* Add clarification for current behavior of H5Get_objtype_by_idx() (#4120)

* Addressed Fortran  issues with promoted integers and reals via compilation flags (#4209)

* addressed issue wit promoted integers and reals

* added option to use mpi_f08

* Summarize the library version table (#4212)

Fixes GH-3773

* Fix URLs (#4210)

Also removed Copyright.html context because it's no longer valid.

* Fix 'make check-vfd' target for Autotools (#4211)

Changes Autotools testing to use HDF5_TEST_DRIVER environment
variable to avoid running tests that don't work well with several
VFDs

Restores old h5_get_vfd_fapl() testing function to setup a FAPL
with a particular VFD

Adds a macro for the default VFD name

* Revert "Addressed Fortran  issues with promoted integers and reals via compil…" (#4220)

This reverts commit 06c42ff.

* Backup and clear CMAKE_C_FLAGS before performing _Float16 configure checks (#4217)

* Fix broken links (#4224)

* Fix broken URLs in documentation (#4214)

Fixes GH-3881 partially.  There are pages that need to be recreated.

* Avoid file size checks in C++ testhdf5 for certain VFDs (#4226)

* Fix an issue with type size assumptions in h5dumpgentest (#4222)

* Fix issue with -Werror cleanup sed command in configure.ac (#4223)

* Fix Java JNI warnings (#4229)

* Rework snapshots/release workflows for consistent args (#4227)

* Fixed a cache assert with too-large metadata objects (#4231)

If the library tries to load a metadata object that is above the
library's hard-coded limits, the size will trip an assert in debug
builds. In HDF5 1.14.4, this can happen if you create a very large
number of links in an old-style group that uses local heaps.

The library will now emit a normal error when it tries to load a
metadata object that is too large.

Partially addresses GitHub #3762

* Set DXPL in API context for native VOL attribute I/O calls (#4228)

* Initialize a variable in C++ testhdf5's tattr.cpp (#4232)

* Addressed Fortran issues with promoted integers and reals via compilation flags, part 2 (#4221)

* addressed issue wit promoted integers and reals

* fixed h5fcreate_f

* added option to use mpi_f08

* change the kind of logical in the parallel tests

* addressed missing return value from callback

* Use cp -rp in test_plugin.sh (#4233)

When building with debug symbols on MacOS, the cp -p commands in
test_plugin.sh will attempt to copy the .dSYM directories with
debugging info, which will fail since -r is missing.

Using cp -rp is harmless and allows the test to run

Fixes HDFFV-10542

* Clean up types in h5test.c (#4235)

Reduces warnings on 32-bit and LLP64 systems

* Fix example links (#4237)

* Fix links md files (#4239)

* Add markdown link checker action (#4219)

* Match minimum CMake version to 3.18 (#4215)
@jhendersonHDF jhendersonHDF deleted the ctx_dxpl_attr_io branch March 27, 2024 03:24
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) Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
Status: Needs Merged
Development

Successfully merging this pull request may close these issues.

4 participants