Skip to content

Commit

Permalink
Pause recording errors instead of clearing the error stack (HDFGroup#…
Browse files Browse the repository at this point in the history
…4475)

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 and lrknox committed Jul 2, 2024
1 parent 3f9a03b commit d2a3193
Show file tree
Hide file tree
Showing 23 changed files with 1,437 additions and 1,884 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
971 changes: 14 additions & 957 deletions release_docs/RELEASE.txt

Large diffs are not rendered by default.

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 d2a3193

Please sign in to comment.