summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVailin Choi <vchoi@jam.ad.hdfgroup.org>2018-12-28 22:49:11 (GMT)
committerVailin Choi <vchoi@jam.ad.hdfgroup.org>2018-12-28 22:49:11 (GMT)
commitf808c108ed0315f115a7c69cbd8ee95032a64b34 (patch)
tree8f196f0f849958019b496027e00a87ed79b0d119
parentba095e6a53a919c9753463c3800a26b180b7a135 (diff)
downloadhdf5-f808c108ed0315f115a7c69cbd8ee95032a64b34.zip
hdf5-f808c108ed0315f115a7c69cbd8ee95032a64b34.tar.gz
hdf5-f808c108ed0315f115a7c69cbd8ee95032a64b34.tar.bz2
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.
-rw-r--r--MANIFEST3
-rw-r--r--configure.ac2
-rw-r--r--release_docs/RELEASE.txt13
-rw-r--r--src/H5Adense.c34
-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, 315 insertions, 20 deletions
diff --git a/MANIFEST b/MANIFEST
index 8783d84..cc2ddc0 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -996,6 +996,7 @@
./test/filespace_1_6.h5
./test/freespace.c
./test/filenotclosed.c
+./test/del_many_dense_attrs.c
./test/file_image.c
./test/file_image_core_test.h5
./test/fill_old.h5
@@ -1103,7 +1104,7 @@
./test/testcheck_version.sh.in
./test/testerror.sh.in
./test/testlinks_env.sh.in
-./test/test_filenotclosed.sh.in
+./test/testabort_fail.sh.in
./test/test_filter_plugin.sh.in
./test/testflushrefresh.sh.in
./test/testframe.c
diff --git a/configure.ac b/configure.ac
index b761042..a583353 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3406,7 +3406,7 @@ AC_CONFIG_FILES([src/libhdf5.settings
test/testlinks_env.sh
test/testswmr.sh
test/testvdsswmr.sh
- test/test_filenotclosed.sh
+ test/testabort_fail.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 024db9a..37e863b 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -243,6 +243,19 @@ Bug Fixes since HDF5-1.10.3 release
Library
-------
+ - Deleting attributes in densed storage
+
+ The library abort with "infinite loop closing library" after
+ attributes in densed 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
diff --git a/src/H5Adense.c b/src/H5Adense.c
index 5bed82d..18cdfe9 100644
--- a/src/H5Adense.c
+++ b/src/H5Adense.c
@@ -300,18 +300,48 @@ 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);
+ /*
+ * 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 ed6bacc..a82fe82 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -374,6 +374,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 26faba6..575eb6d 100644
--- a/test/CMakeTests.cmake
+++ b/test/CMakeTests.cmake
@@ -517,6 +517,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
@@ -793,6 +794,7 @@ set_tests_properties (H5TEST-tcheck_version-release PROPERTIES
# atomic_reader
# links_env
# filenotclosed
+# del_many_dense_attrs
# flushrefresh
##############################################################################
# autotools script tests
@@ -800,7 +802,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.
@@ -827,6 +829,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 1526eca..0bb7410 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
@@ -220,6 +222,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..5ca51d9
--- /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 densed 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 densed 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..90f8077 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 densed 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