Merge pull request #1414 in HDFFV/hdf5 from ~VCHOI/my_hdf5_fork:develop to develop

* commit 'd3dff6efe6f769b219f9dcccebd057afe75ed3c7':
  Correction based on code review.
  Changes based on feedback from pull request.
  Fix for HDFFV-10659: The library abort with "infinite loop closing library" after deleting attributes in densed storage. The fix: When deleting attribute nodes from the name index v2 B-tree, if an attribute is found in the intermediate B-tree nodes, which may be merged/redistributed in the process, we need to free the dynamically allocated spaces for the intermediate decoded attribute.
This commit is contained in:
Vailin Choi 2019-01-02 13:26:58 -06:00
commit c70498f7a9
10 changed files with 317 additions and 21 deletions

View File

@ -968,6 +968,7 @@
./test/cross_read.c
./test/dangle.c
./test/deflate.h5
./test/del_many_dense_attrs.c
./test/direct_chunk.c
./test/dsets.c
./test/dt_arith.c
@ -1102,10 +1103,10 @@
./test/tcheck_version.c
./test/tconfig.c
./test/tcoords.c
./test/testabort_fail.sh.in
./test/testcheck_version.sh.in
./test/testerror.sh.in
./test/testlinks_env.sh.in
./test/test_filenotclosed.sh.in
./test/test_filter_plugin.sh.in
./test/test_filters_le.h5
./test/test_filters_be.h5

View File

@ -3399,6 +3399,7 @@ AC_CONFIG_FILES([src/libhdf5.settings
src/Makefile
test/Makefile
test/H5srcdir_str.h
test/testabort_fail.sh
test/testcheck_version.sh
test/testerror.sh
test/testflushrefresh.sh
@ -3406,7 +3407,6 @@ AC_CONFIG_FILES([src/libhdf5.settings
test/testlinks_env.sh
test/testswmr.sh
test/testvdsswmr.sh
test/test_filenotclosed.sh
test/test_filter_plugin.sh
test/test_usecases.sh
test/test_vol_plugin.sh

View File

@ -1,4 +1,4 @@
HDF5 version 1.11.4 currently under development
version 1.11.4 currently under development
================================================================================
@ -243,6 +243,19 @@ Bug Fixes since HDF5-1.10.3 release
Library
-------
- Deleting attributes in dense storage
The library aborts with "infinite loop closing library" after
attributes in dense storage are created and then deleted.
When deleting the attribute nodes from the name index v2 B-tree,
if an attribute is found in the intermediate B-tree nodes,
which may be merged/redistributed in the process, we need to
free the dynamically allocated spaces for the intermediate
decoded attribute.
(VC - 2018/12/26, HDFFV-10659)
- Allow H5detect and H5make_libsettings to take a file as an argument.
Rather than only writing to stdout, add a command argument to name

View File

@ -300,18 +300,49 @@ static herr_t
H5A__dense_fnd_cb(const H5A_t *attr, hbool_t *took_ownership, void *_user_attr)
{
H5A_t const **user_attr = (H5A_t const **)_user_attr; /* User data from v2 B-tree attribute lookup */
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_STATIC_NOERR
FUNC_ENTER_STATIC
/* Check arguments */
HDassert(attr);
HDassert(user_attr);
HDassert(took_ownership);
/*
* If there is an attribute already stored in "user_attr",
* we need to free the dynamially allocated spaces for the
* attribute, otherwise we got infinite loop closing library due to
* outstanding allocation. (HDFFV-10659)
*
* This callback is used by H5A__dense_remove() to close/free the
* attribute stored in "user_attr" (via H5O__msg_free_real()) after
* the attribute node is deleted from the name index v2 B-tree.
* The issue is:
* When deleting the attribute node from the B-tree,
* if the attribute is found in the intermediate B-tree nodes,
* which may be merged/redistributed, we need to free the dynamically
* allocated spaces for the intermediate decoded attribute.
*/
if(*user_attr != NULL) {
H5A_t *old_attr = *user_attr;
if(old_attr->shared) {
/* Free any dynamically allocated items */
if(H5A__free(old_attr) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release attribute info")
/* Destroy shared attribute struct */
old_attr->shared = H5FL_FREE(H5A_shared_t, old_attr->shared);
} /* end if */
old_attr = H5FL_FREE(H5A_t, old_attr);
} /* end if */
/* Take over attribute ownership */
*user_attr = attr;
*took_ownership = TRUE;
FUNC_LEAVE_NOAPI(SUCCEED)
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5A__dense_fnd_cb() */

View File

@ -374,6 +374,7 @@ set (H5_CHECK_TESTS
atomic_reader
links_env
filenotclosed
del_many_dense_attrs
flushrefresh
)

View File

@ -520,6 +520,7 @@ set (test_CLEANFILES
flushrefresh_VERIFICATION_CHECKPOINT2
flushrefresh_VERIFICATION_DONE
filenotclosed.h5
del_many_dense_attrs.h5
atomic_data
accum_swmr_big.h5
ohdr_swmr.h5
@ -796,6 +797,7 @@ set_tests_properties (H5TEST-tcheck_version-release PROPERTIES
# atomic_reader
# links_env
# filenotclosed
# del_many_dense_attrs
# flushrefresh
##############################################################################
# autotools script tests
@ -803,7 +805,7 @@ set_tests_properties (H5TEST-tcheck_version-release PROPERTIES
# NOT CONVERTED accum_swmr_reader is used by accum.c.
# NOT CONVERTED atomic_writer and atomic_reader are standalone programs.
# links_env is used by testlinks_env.sh
# filenotclosed is used by test_filenotclosed.sh
# filenotclosed and del_many_dense_attrs are used by testabort_fail.sh
# NOT CONVERTED flushrefresh is used by testflushrefresh.sh.
# NOT CONVERTED use_append_chunk, use_append_mchunks and use_disable_mdc_flushes are used by test_usecases.sh
# NOT CONVERTED swmr_* files (besides swmr.c) are used by testswmr.sh.
@ -830,6 +832,23 @@ set_tests_properties (H5TEST-filenotclosed PROPERTIES
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
)
#-- Adding test for del_many_dense_attrs
add_test (
NAME H5TEST-clear-del_many_dense_attrs-objects
COMMAND ${CMAKE_COMMAND}
-E remove
del_many_dense_attrs.h5
WORKING_DIRECTORY
${HDF5_TEST_BINARY_DIR}/H5TEST
)
set_tests_properties (H5TEST-clear-del_many_dense_attrs-objects PROPERTIES FIXTURES_SETUP del_many_dense_attrs_clear_objects)
add_test (NAME H5TEST-del_many_dense_attrs COMMAND $<TARGET_FILE:del_many_dense_attrs>)
set_tests_properties (H5TEST-del_many_dense_attrs PROPERTIES
FIXTURES_REQUIRED del_many_dense_attrs_clear_objects
ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST"
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
)
#-- Adding test for err_compat
if (HDF5_ENABLE_DEPRECATED_SYMBOLS)
add_test (NAME H5TEST-clear-err_compat-objects

View File

@ -29,12 +29,13 @@ AM_CPPFLAGS+=-I$(top_srcdir)/src -I$(top_builddir)/src
# testflushrefresh.sh: flushrefresh
# testswmr.sh: swmr*
# testvdsswmr.sh: vds_swmr*
# test_filenotclosed.sh: filenotclosed.c
# testabort_fail.sh: filenotclosed.c and del_many_dense_attrs.c
# test_filter_plugin.sh: filter_plugin.c
# test_usecases.sh: use_append_chunk, use_append_mchunks, use_disable_mdc_flushes
TEST_SCRIPT = testerror.sh testlibinfo.sh testcheck_version.sh testlinks_env.sh test_filenotclosed.sh\
testswmr.sh testvdsswmr.sh testflushrefresh.sh test_usecases.sh
SCRIPT_DEPEND = error_test$(EXEEXT) err_compat$(EXEEXT) links_env$(EXEEXT) filenotclosed$(EXEEXT) \
TEST_SCRIPT = testerror.sh testlibinfo.sh testcheck_version.sh testlinks_env.sh \
testswmr.sh testvdsswmr.sh testflushrefresh.sh test_usecases.sh testabort_fail.sh
SCRIPT_DEPEND = error_test$(EXEEXT) err_compat$(EXEEXT) links_env$(EXEEXT) \
filenotclosed$(EXEEXT) del_many_dense_attrs$(EXEEXT) \
flushrefresh$(EXEEXT) use_append_chunk$(EXEEXT) use_append_mchunks$(EXEEXT) use_disable_mdc_flushes$(EXEEXT) \
swmr_generator$(EXEEXT) swmr_reader$(EXEEXT) swmr_writer$(EXEEXT) \
swmr_remove_reader$(EXEEXT) swmr_remove_writer$(EXEEXT) swmr_addrem_writer$(EXEEXT) \
@ -68,7 +69,7 @@ TEST_PROG= testhdf5 \
# accum_swmr_reader is used by accum.c.
# atomic_writer and atomic_reader are standalone programs.
# links_env is used by testlinks_env.sh
# filenotclosed is used by test_filenotclosed.sh
# filenotclosed and del_many_dense_attrs are used by testabort_fail.sh
# flushrefresh is used by testflushrefresh.sh.
# use_append_chunk, use_append_mchunks and use_disable_mdc_flushes are used by test_usecases.sh
# swmr_* files (besides swmr.c) are used by testswmr.sh.
@ -78,7 +79,8 @@ TEST_PROG= testhdf5 \
# and this lets automake keep all its test programs in one place.
check_PROGRAMS=$(TEST_PROG) error_test err_compat tcheck_version \
testmeta accum_swmr_reader atomic_writer atomic_reader \
links_env filenotclosed flushrefresh use_append_chunk use_append_mchunks use_disable_mdc_flushes \
links_env filenotclosed del_many_dense_attrs flushrefresh \
use_append_chunk use_append_mchunks use_disable_mdc_flushes \
swmr_generator swmr_start_write swmr_reader swmr_writer swmr_remove_reader \
swmr_remove_writer swmr_addrem_writer swmr_sparse_reader swmr_sparse_writer \
swmr_check_compat_vfd vds_swmr_gen vds_swmr_reader vds_swmr_writer
@ -221,7 +223,7 @@ use_disable_mdc_flushes_SOURCES=use_disable_mdc_flushes.c
# Temporary files.
DISTCLEANFILES=testerror.sh testlibinfo.sh testcheck_version.sh testlinks_env.sh test_filter_plugin.sh \
testswmr.sh testvdsswmr.sh test_usecases.sh testflushrefresh.sh test_filenotclosed.sh \
testswmr.sh testvdsswmr.sh test_usecases.sh testflushrefresh.sh testabort_fail.sh \
test_vol_plugin.sh
include $(top_srcdir)/config/conclude.am

View File

@ -171,6 +171,7 @@ if (UNIX)
# atomic_writer
# atomic_reader
# filenotclosed
# del_many_dense_attrs
# flushrefresh
##############################################################################
# autotools script tests
@ -178,7 +179,7 @@ if (UNIX)
# NOT CONVERTED accum_swmr_reader is used by accum.c.
# NOT CONVERTED atomic_writer and atomic_reader are standalone programs.
# links_env is used by testlinks_env.sh
# filenotclosed is used by test_filenotclosed.sh
# filenotclosed and del_many_dense_attrs are used by testabort_fail.sh
# NOT CONVERTED flushrefresh is used by testflushrefresh.sh.
# NOT CONVERTED use_append_chunk, use_append_mchunks and use_disable_mdc_flushes are used by test_usecases.sh
# NOT CONVERTED swmr_* files (besides swmr.c) are used by testswmr.sh.

203
test/del_many_dense_attrs.c Normal file
View File

@ -0,0 +1,203 @@
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* Copyright by The HDF Group. *
* Copyright by the Board of Trustees of the University of Illinois. *
* All rights reserved. *
* *
* This file is part of HDF5. The full HDF5 copyright notice, including *
* terms governing use, modification, and redistribution, is contained in *
* the COPYING file, which can be found at the root of the source code *
* distribution tree, or in https://support.hdfgroup.org/ftp/HDF5/releases. *
* If you do not have access to either file, you may request a copy from *
* help@hdfgroup.org. *
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/*
* Purpose: Test to verify that the infinite loop closing library/abort failure
* is fixed when the application creates and removes dense attributes
* (See HDFFV-10659).
*/
#include "h5test.h"
/* The test file name */
const char *FILENAME[] = {
"del_many_dense_attrs",
NULL
};
#define ATTR_COUNT 64 /* The number of attributes */
/*-------------------------------------------------------------------------
* Function: catch_signal
*
* Purpose: The signal handler to catch the SIGABRT signal.
*
* Return: No return
*
* Programmer: Vailin Choi
*
*-------------------------------------------------------------------------
*/
static void catch_signal(int H5_ATTR_UNUSED signo)
{
HDexit(1);
} /* catch_signal() */
/*-------------------------------------------------------------------------
* Function: main
*
* Purpose: Test to verify that the infinite loop closing library/abort failure
* is fixed when the application creates and removes dense attributes
* (See HDFFV-10659).
*
* Return: Success: exit(EXIT_SUCCESS)
* Failure: exit(EXIT_FAILURE)
*
* Programmer: Vailin Choi; Dec 2018
*
*-------------------------------------------------------------------------
*/
int
main(void)
{
hid_t fid = -1; /* HDF5 File ID */
hid_t gid = -1; /* Group ID */
hid_t sid = -1; /* Dataspace ID */
hid_t aid = -1; /* Attribute ID */
hid_t tid = -1; /* Datatype ID */
hid_t fapl = -1; /* File access property lists */
hid_t gcpl = -1; /* Group creation property list */
char aname[50]; /* Name of attribute */
char *basename="attr"; /* Name prefix for attribute */
char filename[100]; /* File name */
int i; /* Local index variable */
/* Testing setup */
h5_reset();
/* To exit from the file for SIGABRT signal */
if(HDsignal(SIGABRT, catch_signal) == SIG_ERR)
TEST_ERROR
fapl = h5_fileaccess();
h5_fixname(FILENAME[0], fapl, filename, sizeof(filename));
/* Set to latest format */
if(H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST) < 0)
TEST_ERROR
/* Create the file */
if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0)
TEST_ERROR
/* Close the file */
if(H5Fclose(fid) < 0)
TEST_ERROR
/* Re-open the file */
if((fid = H5Fopen(filename, H5F_ACC_RDWR, fapl)) < 0)
TEST_ERROR
/* Create the group creation property list */
if((gcpl = H5Pcreate(H5P_GROUP_CREATE)) < 0)
TEST_ERROR
/* Set to use dense storage for all attributes on the group */
if(H5Pset_attr_phase_change(gcpl, 0, 0) < 0)
TEST_ERROR
/* Create the group in the file */
if((gid = H5Gcreate2(fid, "group", H5P_DEFAULT, gcpl, H5P_DEFAULT)) < 0)
TEST_ERROR
/* Create dataspace */
if((sid = H5Screate(H5S_SCALAR)) < 0)
TEST_ERROR
/* Get a copy of the datatype */
if((tid = H5Tcopy(H5T_NATIVE_FLOAT)) < 0)
TEST_ERROR
/* Create attributes in the group */
for(i = ATTR_COUNT; i >= 0; i--) {
/* Set up the attribute name */
HDsprintf(aname, "%s%d", basename, i);
/* Create the attribute */
if((aid = H5Acreate2(gid, aname, tid, sid, H5P_DEFAULT, H5P_DEFAULT)) < 0)
TEST_ERROR
/* Write to the attribute */
if(H5Awrite(aid, tid, &i) < 0)
TEST_ERROR
/* Close the attribute */
if(H5Aclose(aid) < 0)
TEST_ERROR
}
/* Close the datatype */
if(H5Tclose(tid) < 0)
TEST_ERROR
/* Close the dataspace */
if(H5Sclose(sid) < 0)
TEST_ERROR
/* Close the group */
if(H5Gclose(gid) < 0)
TEST_ERROR
/* Close the group creation property list */
if(H5Pclose(gcpl) < 0)
TEST_ERROR
/* Close the file */
if(H5Fclose(fid) < 0)
TEST_ERROR
/* Re-open the file */
if((fid = H5Fopen(filename, H5F_ACC_RDWR, fapl)) < 0)
TEST_ERROR
/* Open the group */
if((gid = H5Gopen(fid, "group", H5P_DEFAULT)) < 0)
TEST_ERROR
/* Delete the attributes */
for (i = 0; i <= ATTR_COUNT; i++) {
/* Set up the attribute name */
HDsprintf(aname, "%s%d", basename, i);
/* Delete the attribute */
if(H5Adelete(gid, aname) < 0)
TEST_ERROR
} /* end for */
/* Close the group */
if(H5Gclose(gid) < 0)
TEST_ERROR
/* Close the file */
if(H5Fclose(fid) < 0)
TEST_ERROR
h5_cleanup(FILENAME, fapl);
return(EXIT_SUCCESS);
error:
H5E_BEGIN_TRY {
H5Gclose(gid);
H5Sclose(sid);
H5Tclose(tid);
H5Aclose(aid);
H5Fclose(fid);
H5Pclose(gcpl);
H5Pclose(fapl);
} H5E_END_TRY
return EXIT_FAILURE;
}

View File

@ -13,6 +13,9 @@
#
# Test to verify that the assertion/abort failure is fixed when the application
# does not close the file. (See HDFFV-10160)
#
# Test to verify that the infinite loop closing library/abort failure is fixed
# when the application creates and removes dense attributes (See HDFFV-10659)
srcdir=@srcdir@
@ -20,22 +23,44 @@ nerrors=0
##############################################################################
##############################################################################
### T H E T E S T ###
### T H E T E S T S ###
##############################################################################
##############################################################################
#
#
echo "Testing file not closed assertion/abort failure"
TEST_NAME=filenotclosed # The test name
TEST_BIN=`pwd`/$TEST_NAME # The path of the test binary
TEST_BIN=`pwd`/$TEST_NAME # The path of the test binary
#
# Run the test
#$RUNSERIAL $TEST_BIN >/dev/null 2>&1
$RUNSERIAL $TEST_BIN 2>&1
$RUNSERIAL $TEST_BIN >/dev/null 2>&1
exitcode=$?
if [ $exitcode -eq 0 ]; then
echo "Test PASSED"
else
echo "Test FAILED"
nerrors="`expr $nerrors + 1`"
echo "***Error encountered***"
fi
exit $nerrors
#
#
echo "Testing infinite loop closing library/abort failure"
TEST_NAME=del_many_dense_attrs # The test name
TEST_BIN=`pwd`/$TEST_NAME # The path of the test binary
# Run the test
$RUNSERIAL $TEST_BIN >/dev/null 2>&1
exitcode=$?
if [ $exitcode -eq 0 ]; then
echo "Test PASSED"
else
echo "Test FAILED"
nerrors="`expr $nerrors + 1`"
fi
#
#
if test $nerrors -eq 0 ; then
echo "All tests for abort failure passed."
exit 0
else
echo "Tests for abort failure failed with $nerrors errors."
exit 1
fi