Skip to content

Commit

Permalink
Pause recording errors instead of clearing the error stack (#4475)
Browse files Browse the repository at this point in the history
An internal capability that's similar to the H5E_BEGIN_TRY / H5E_END_TRY
macros in H5Epublic.h, but more efficient since we can avoid pushing errors on
the stack entirely (and those macros use public API routines).

This capability (and other techniques) can be used to remove use of
H5E_clear_stack() and H5E_BEGIN_TRY / H5E_END_TRY within library routines.

We want to remove H5E_clear_stack() because it can trigger calls to the H5I
interface from within the H5E code, which creates a great deal of complexity
for threadsafe code.  And we want to remove H5E_BEGIN_TRY / H5E_END_TRY's
because they make public API calls from within the library code.

Also some other minor tidying in routines related to removing the use of
H5E_clear_stack() and H5E_BEGIN_TRY / H5E_END_TRY from H5Fint.c
  • Loading branch information
qkoziol authored Jun 18, 2024
1 parent 82a0ff1 commit 850d6c8
Show file tree
Hide file tree
Showing 23 changed files with 1,434 additions and 929 deletions.
6 changes: 4 additions & 2 deletions bin/format_source
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ find . \( -type d -path ./config -prune -and -not -path ./config \) \
-name H5LTanalyze.c \
-or -name H5LTparse.c \
-or -name H5LTparse.h \
-or -name H5Epubgen.h \
-or -name H5Edefin.h \
-or -name H5Einit.h \
-or -name H5Emajdef.h \
-or -name H5Emindef.h \
-or -name H5Epubgen.h \
-or -name H5Eterm.h \
-or -name H5Edefin.h \
-or -name H5version.h \
-or -name H5overflow.h \
\) \) \
Expand Down
13 changes: 11 additions & 2 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ New Features
CMake supports two main files, CMakePresets.json and CMakeUserPresets.json,
that allow users to specify common configure options and share them with others.
HDF added a CMakePresets.json file of a typical configuration and support
file, config/cmake-presets/hidden-presets.json.
file, config/cmake-presets/hidden-presets.json.
Also added a section to INSTALL_CMake.txt with very basic explanation of the
process to use CMakePresets.

Expand Down Expand Up @@ -431,6 +431,15 @@ New Features

Library:
--------
- Added new routines for interacting with error stacks: H5Epause_stack,
H5Eresume_stack, and H5Eis_paused. These routines can be used to
indicate that errors from a call to an HDF5 routine should not be
pushed on to an error stack. Primarily targeted toward 3rd-party
developers of Virtual File Drivirs (VFDs) and Virtual Object Layer (VOL)
connectors, these routines allow developers to perform "speculative"
operations (such as trying to open a file or object) without requiring
that the error stack be cleared after a speculative operation fails.

- H5Pset_external() now uses HDoff_t, which is always a 64-bit type

The H5Pset_external() call took an off_t parameter in HDF5 1.14.x and
Expand Down Expand Up @@ -581,7 +590,7 @@ New Features
to filtered datasets. The Subfiling VFD now properly handles vector
I/O requests in their entirety, resulting in fewer I/O calls, improved
vector I/O performance and improved vector I/O memory efficiency.

- Added a simple cache to the read-only S3 (ros3) VFD

The read-only S3 VFD now caches the first N bytes of a file stored
Expand Down
10 changes: 4 additions & 6 deletions src/H5Dvirtual.c
Original file line number Diff line number Diff line change
Expand Up @@ -875,16 +875,14 @@ H5D__virtual_open_source_dset(const H5D_t *vdset, H5O_storage_virtual_ent_t *vir
intent = H5F_INTENT(vdset->oloc.file);

/* Try opening the file */
src_file = H5F_prefix_open_file(vdset->oloc.file, H5F_PREFIX_VDS, vdset->shared->vds_prefix,
source_dset->file_name, intent,
vdset->shared->layout.storage.u.virt.source_fapl);
if (H5F_prefix_open_file(true, &src_file, vdset->oloc.file, H5F_PREFIX_VDS, vdset->shared->vds_prefix,
source_dset->file_name, intent,
vdset->shared->layout.storage.u.virt.source_fapl) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTOPENFILE, FAIL, "can't try opening file");

/* If we opened the source file here, we should close it when leaving */
if (src_file)
src_file_open = true;
else
/* Reset the error stack */
H5E_clear_stack();
} /* end if */
else
/* Source file is ".", use the virtual dataset's file */
Expand Down
179 changes: 147 additions & 32 deletions src/H5E.c
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,11 @@ H5Epush2(hid_t err_stack, const char *file, const char *func, unsigned line, hid
/* Don't clear the error stack! :-) */
FUNC_ENTER_API_NOCLEAR(FAIL)

if (err_stack == H5E_DEFAULT)
estack = NULL;
/* Check for 'default' error stack */
if (err_stack == H5E_DEFAULT) {
if (NULL == (estack = H5E__get_my_stack()))
HGOTO_ERROR(H5E_ERROR, H5E_CANTGET, FAIL, "can't get the default error stack");
}
else {
/* Only clear the error stack if it's not the default stack */
H5E_clear_stack();
Expand All @@ -543,35 +546,33 @@ H5Epush2(hid_t err_stack, const char *file, const char *func, unsigned line, hid
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a error stack ID");
} /* end else */

/* Note that the variable-argument parsing for the format is identical in
* the H5E_printf_stack() routine - correct errors and make changes in both
* places. -QAK
*/

/* Format the description */
va_start(ap, fmt);
va_started = true;

/* Duplicate string information */
if (NULL == (tmp_file = strdup(file)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate file string");
if (NULL == (tmp_func = strdup(func)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate function string");

/* Increment refcount on non-library IDs */
if (cls_id != H5E_ERR_CLS_g)
if (H5I_inc_ref(cls_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment class ID");
if (maj_id < H5E_first_maj_id_g || maj_id > H5E_last_maj_id_g)
if (H5I_inc_ref(maj_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment major error ID");
if (min_id < H5E_first_min_id_g || min_id > H5E_last_min_id_g)
if (H5I_inc_ref(min_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment minor error ID");

/* Push the error on the stack */
if (H5E__push_stack(estack, true, tmp_file, tmp_func, line, cls_id, maj_id, min_id, fmt, &ap) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't push error on stack");
/* Check if error reporting is paused for this stack */
if (!estack->paused) {
/* Start the variable-argument parsing */
va_start(ap, fmt);
va_started = true;

/* Duplicate string information */
if (NULL == (tmp_file = strdup(file)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate file string");
if (NULL == (tmp_func = strdup(func)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate function string");

/* Increment refcount on non-library IDs */
if (cls_id != H5E_ERR_CLS_g)
if (H5I_inc_ref(cls_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment class ID");
if (maj_id < H5E_first_maj_id_g || maj_id > H5E_last_maj_id_g)
if (H5I_inc_ref(maj_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment major error ID");
if (min_id < H5E_first_min_id_g || min_id > H5E_last_min_id_g)
if (H5I_inc_ref(min_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment minor error ID");

/* Push the error on the stack */
if (H5E__push_stack(estack, true, tmp_file, tmp_func, line, cls_id, maj_id, min_id, fmt, &ap) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't push error on stack");
}

done:
if (va_started)
Expand Down Expand Up @@ -865,7 +866,6 @@ H5Eappend_stack(hid_t dst_stack_id, hid_t src_stack_id, hbool_t close_source_sta
H5E_stack_t *dst_stack, *src_stack; /* Error stacks */
herr_t ret_value = SUCCEED; /* Return value */

/* Don't clear the error stack! :-) */
FUNC_ENTER_API(FAIL)

/* Check args */
Expand All @@ -889,3 +889,118 @@ H5Eappend_stack(hid_t dst_stack_id, hid_t src_stack_id, hbool_t close_source_sta
done:
FUNC_LEAVE_API(ret_value)
} /* end H5Eappend_stack() */

/*-------------------------------------------------------------------------
* Function: H5Eis_paused
*
* Purpose: Check if pushing errors on an error stack is paused
*
* Return: Non-negative value on success/Negative on failure
*
*-------------------------------------------------------------------------
*/
herr_t
H5Eis_paused(hid_t stack_id, hbool_t *is_paused)
{
H5E_stack_t *stack; /* Error stack */
herr_t ret_value = SUCCEED; /* Return value */

/* Don't clear the error stack! :-) */
FUNC_ENTER_API_NOCLEAR(FAIL)

/* Get the correct error stack */
if (stack_id == H5E_DEFAULT) {
if (NULL == (stack = H5E__get_my_stack()))
HGOTO_ERROR(H5E_ERROR, H5E_CANTGET, FAIL, "can't get current error stack");
} /* end if */
else {
/* Only clear the error stack if it's not the default stack */
H5E_clear_stack();

/* Get the error stack to operate on */
if (NULL == (stack = (H5E_stack_t *)H5I_object_verify(stack_id, H5I_ERROR_STACK)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an error stack ID");
} /* end else */

/* Check arguments */
if (NULL == is_paused)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "is_paused parameter is NULL");

/* Check if the stack is paused */
*is_paused = (stack->paused > 0);

done:
FUNC_LEAVE_API(ret_value)
} /* end H5Eis_paused() */

/*-------------------------------------------------------------------------
* Function: H5Epause_stack
*
* Purpose: Pause pushing errors on an error stack
*
* Return: Non-negative value on success/Negative on failure
*
*-------------------------------------------------------------------------
*/
herr_t
H5Epause_stack(hid_t stack_id)
{
H5E_stack_t *stack; /* Error stack */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_API(FAIL)

/* Get the correct error stack */
if (stack_id == H5E_DEFAULT) {
if (NULL == (stack = H5E__get_my_stack()))
HGOTO_ERROR(H5E_ERROR, H5E_CANTGET, FAIL, "can't get current error stack");
} /* end if */
else
/* Get the error stack to operate on */
if (NULL == (stack = (H5E_stack_t *)H5I_object_verify(stack_id, H5I_ERROR_STACK)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an error stack ID");

/* Increment pause counter */
stack->paused++;

done:
FUNC_LEAVE_API(ret_value)
} /* end H5Epause_stack() */

/*-------------------------------------------------------------------------
* Function: H5Eresume_stack
*
* Purpose: Resume pushing errors on an error stack
*
* Return: Non-negative value on success/Negative on failure
*
*-------------------------------------------------------------------------
*/
herr_t
H5Eresume_stack(hid_t stack_id)
{
H5E_stack_t *stack; /* Error stack */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_API(FAIL)

/* Get the correct error stack */
if (stack_id == H5E_DEFAULT) {
if (NULL == (stack = H5E__get_my_stack()))
HGOTO_ERROR(H5E_ERROR, H5E_CANTGET, FAIL, "can't get current error stack");
} /* end if */
else
/* Get the error stack to operate on */
if (NULL == (stack = (H5E_stack_t *)H5I_object_verify(stack_id, H5I_ERROR_STACK)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an error stack ID");

/* Check for pause/resume imbalance */
if (0 == stack->paused)
HGOTO_ERROR(H5E_ERROR, H5E_BADRANGE, FAIL, "resuming more than paused");

/* Decrement pause counter */
stack->paused--;

done:
FUNC_LEAVE_API(ret_value)
} /* end H5Eresume_stack() */
49 changes: 29 additions & 20 deletions src/H5Edeprec.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,30 +182,38 @@ H5Eget_minor(H5E_minor_t min)
herr_t
H5Epush1(const char *file, const char *func, unsigned line, H5E_major_t maj, H5E_minor_t min, const char *str)
{
const char *tmp_file; /* Copy of the file name */
const char *tmp_func; /* Copy of the function name */
herr_t ret_value = SUCCEED; /* Return value */
H5E_stack_t *estack; /* Pointer to error stack to modify */
const char *tmp_file; /* Copy of the file name */
const char *tmp_func; /* Copy of the function name */
herr_t ret_value = SUCCEED; /* Return value */

/* Don't clear the error stack! :-) */
FUNC_ENTER_API_NOCLEAR(FAIL)

/* Duplicate string information */
if (NULL == (tmp_file = strdup(file)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate file string");
if (NULL == (tmp_func = strdup(func)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate function string");

/* Increment refcount on non-library IDs */
if (maj < H5E_first_maj_id_g || maj > H5E_last_maj_id_g)
if (H5I_inc_ref(maj, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment major error ID");
if (min < H5E_first_min_id_g || min > H5E_last_min_id_g)
if (H5I_inc_ref(min, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment minor error ID");

/* Push the error on the default error stack */
if (H5E__push_stack(NULL, true, tmp_file, tmp_func, line, H5E_ERR_CLS_g, maj, min, str, NULL) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't push error on stack");
/* Get the 'default' error stack */
if (NULL == (estack = H5E__get_my_stack()))
HGOTO_ERROR(H5E_ERROR, H5E_CANTGET, FAIL, "can't get the default error stack");

/* Check if error reporting is paused for this stack */
if (!estack->paused) {
/* Duplicate string information */
if (NULL == (tmp_file = strdup(file)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate file string");
if (NULL == (tmp_func = strdup(func)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "can't duplicate function string");

/* Increment refcount on non-library IDs */
if (maj < H5E_first_maj_id_g || maj > H5E_last_maj_id_g)
if (H5I_inc_ref(maj, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment major error ID");
if (min < H5E_first_min_id_g || min > H5E_last_min_id_g)
if (H5I_inc_ref(min, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "can't increment minor error ID");

/* Push the error on the default error stack */
if (H5E__push_stack(estack, true, tmp_file, tmp_func, line, H5E_ERR_CLS_g, maj, min, str, NULL) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't push error on stack");
}

done:
FUNC_LEAVE_API(ret_value)
Expand Down Expand Up @@ -259,6 +267,7 @@ H5Eprint1(FILE *stream)
/* Don't clear the error stack! :-) */
FUNC_ENTER_API_NOCLEAR(FAIL)

/* Get the 'default' error stack */
if (NULL == (estack = H5E__get_my_stack()))
HGOTO_ERROR(H5E_ERROR, H5E_CANTGET, FAIL, "can't get current error stack");

Expand Down
Loading

0 comments on commit 850d6c8

Please sign in to comment.