summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVailin Choi <vchoi@jam.ad.hdfgroup.org>2019-01-06 15:25:48 (GMT)
committerVailin Choi <vchoi@jam.ad.hdfgroup.org>2019-01-06 15:25:48 (GMT)
commit489f6fb69711ef7f26f4c13ad863438779f654b8 (patch)
tree847210b7d0b62f1259ba1ea98bc72ff8369ccb36
parent906479d3978adabbacb59da8ec7ffb85d0ddfc2f (diff)
downloadhdf5-489f6fb69711ef7f26f4c13ad863438779f654b8.zip
hdf5-489f6fb69711ef7f26f4c13ad863438779f654b8.tar.gz
hdf5-489f6fb69711ef7f26f4c13ad863438779f654b8.tar.bz2
Fix for HDFFV-10659: The library aborts 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.
-rw-r--r--MANIFEST3
-rw-r--r--configure.ac2
-rw-r--r--release_docs/RELEASE.txt17
-rw-r--r--src/H5Adense.c35
-rw-r--r--test/CMakeLists.txt1
-rw-r--r--test/CMakeTests.cmake21
-rw-r--r--test/Makefile.am16
-rw-r--r--test/ShellTests.cmake3
-rw-r--r--test/del_many_dense_attrs.c203
-rw-r--r--test/testabort_fail.sh.in (renamed from test/test_filenotclosed.sh.in)39
10 files changed, 320 insertions, 20 deletions
diff --git a/MANIFEST b/MANIFEST
index 936bd7e..8e08ba3 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -948,6 +948,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
@@ -1079,10 +1080,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/testflushrefresh.sh.in
./test/testframe.c
diff --git a/configure.ac b/configure.ac
index 572fbe5..4d69027 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3395,6 +3395,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
@@ -3402,7 +3403,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
testpar/Makefile
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 05daa53..288881d 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -1,4 +1,8 @@
+<<<<<<< HEAD
HDF5 version 1.10.5-snap1 currently under development
+=======
+ version 1.11.4 currently under development
+>>>>>>> c70498f... Merge pull request #1414 in HDFFV/hdf5 from ~VCHOI/my_hdf5_fork:develop to develop
================================================================================
@@ -246,6 +250,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)
+
- A bug was discovered in the parallel library where an application
would eventually consume all of the available MPI communicators
when continually writing to a compressed dataset in parallel. This
diff --git a/src/H5Adense.c b/src/H5Adense.c
index 3701022..40a7a9a 100644
--- a/src/H5Adense.c
+++ b/src/H5Adense.c
@@ -302,20 +302,51 @@ 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() */
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index ce39506..fa303ed 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -342,6 +342,7 @@ set (H5_CHECK_TESTS
atomic_reader
links_env
filenotclosed
+ del_many_dense_attrs
flushrefresh
)
diff --git a/test/CMakeTests.cmake b/test/CMakeTests.cmake
index ba64964..881bdb8 100644
--- a/test/CMakeTests.cmake
+++ b/test/CMakeTests.cmake
@@ -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
@@ -795,6 +796,7 @@ set_tests_properties (H5TEST-tcheck_version-release PROPERTIES
# atomic_reader
# links_env
# filenotclosed
+# del_many_dense_attrs
# flushrefresh
##############################################################################
# autotools script tests
@@ -802,7 +804,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.
@@ -829,6 +831,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
diff --git a/test/Makefile.am b/test/Makefile.am
index 6a057c1..c4fb876 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -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
@@ -209,6 +211,6 @@ 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
include $(top_srcdir)/config/conclude.am
diff --git a/test/ShellTests.cmake b/test/ShellTests.cmake
index 58dc85d..98f3daf 100644
--- a/test/ShellTests.cmake
+++ b/test/ShellTests.cmake
@@ -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.
diff --git a/test/del_many_dense_attrs.c b/test/del_many_dense_attrs.c
new file mode 100644
index 0000000..bbae48d
--- /dev/null
+++ b/test/del_many_dense_attrs.c
@@ -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;
+}
diff --git a/test/test_filenotclosed.sh.in b/test/testabort_fail.sh.in
index 0b43c5b..925d8a4 100644
--- a/test/test_filenotclosed.sh.in
+++ b/test/testabort_fail.sh.in
@@ -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