summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorvchoi-hdfgroup <55293060+vchoi-hdfgroup@users.noreply.github.com>2023-02-26 18:07:52 (GMT)
committerGitHub <noreply@github.com>2023-02-26 18:07:52 (GMT)
commit063a61c36b189bb9b8249f495043a32967eda9d7 (patch)
tree1890addb72c5f09bb35b1a4818b99aa0d17bd925 /src
parenta7dd6452a0be68e3bd3af74dd583f959a9d6e65c (diff)
downloadhdf5-063a61c36b189bb9b8249f495043a32967eda9d7.zip
hdf5-063a61c36b189bb9b8249f495043a32967eda9d7.tar.gz
hdf5-063a61c36b189bb9b8249f495043a32967eda9d7.tar.bz2
Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (#2291) (#2496)
* Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC) producing a core dump. When h5debug closes the corrupted file, the library calls H5F__dest() which performs all the closing operations for the file "f" (H5F_t *) but just keeping note of errors in "ret_value" all the way till the end of the routine. The user-provided corrupted file has an illegal file size causing failure when reading the image during the closing process. At the end of this routine it sets f->shared to NULL and then frees "f". This is done whether there is error or not in "ret_value". Due to the failure in reading the file earlier, the routine then returns error. The error return from H5F__dest() causes the file object "f" not being removed from the ID node table. When the library finally exits, it will try to close the file objects in the table. This causes assert failure when H5F_ID_EXISTS(f) or H5F_NREFS(f). Fix: a) H5F_dest(): free the f only when there is no error in "ret_value" at the end of the routine. b) H5VL__native_file_close(): if f->shared is NULL, free "f"; otherwise, perform closing on "f" as before. c) h5debug.c main(): track error return from H5Fclose(). * Committing clang-format changes Co-authored-by: vchoi <vchoi@jelly.ad.hdfgroup.org> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Diffstat (limited to 'src')
-rw-r--r--src/H5Fint.c4
-rw-r--r--src/H5VLnative_file.c52
2 files changed, 32 insertions, 24 deletions
diff --git a/src/H5Fint.c b/src/H5Fint.c
index dc86390..1bdf09b 100644
--- a/src/H5Fint.c
+++ b/src/H5Fint.c
@@ -1648,7 +1648,9 @@ H5F__dest(H5F_t *f, hbool_t flush)
if (H5FO_top_dest(f) < 0)
HDONE_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "problems closing file")
f->shared = NULL;
- f = H5FL_FREE(H5F_t, f);
+
+ if (ret_value >= 0)
+ f = H5FL_FREE(H5F_t, f);
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5F__dest() */
diff --git a/src/H5VLnative_file.c b/src/H5VLnative_file.c
index 3803e6b..a1a5c7d 100644
--- a/src/H5VLnative_file.c
+++ b/src/H5VLnative_file.c
@@ -813,29 +813,35 @@ H5VL__native_file_close(void *file, hid_t H5_ATTR_UNUSED dxpl_id, void H5_ATTR_U
FUNC_ENTER_PACKAGE
/* This routine should only be called when a file ID's ref count drops to zero */
- HDassert(H5F_ID_EXISTS(f));
-
- /* Flush file if this is the last reference to this id and we have write
- * intent, unless it will be flushed by the "shared" file being closed.
- * This is only necessary to replicate previous behaviour, and could be
- * disabled by an option/property to improve performance.
- */
- if ((H5F_NREFS(f) > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) {
- /* Get the file ID corresponding to the H5F_t struct */
- if (H5I_find_id(f, H5I_FILE, &file_id) < 0 || H5I_INVALID_HID == file_id)
- HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "invalid atom")
-
- /* Get the number of references outstanding for this file ID */
- if ((nref = H5I_get_ref(file_id, FALSE)) < 0)
- HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "can't get ID ref count")
- if (nref == 1)
- if (H5F__flush(f) < 0)
- HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache")
- } /* end if */
-
- /* Close the file */
- if (H5F__close(f) < 0)
- HGOTO_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "can't close file")
+ HDassert(f->shared == NULL || H5F_ID_EXISTS(f));
+
+ if (f->shared == NULL)
+ f = H5FL_FREE(H5F_t, f);
+
+ else {
+
+ /* Flush file if this is the last reference to this id and we have write
+ * intent, unless it will be flushed by the "shared" file being closed.
+ * This is only necessary to replicate previous behaviour, and could be
+ * disabled by an option/property to improve performance.
+ */
+ if ((H5F_NREFS(f) > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) {
+ /* Get the file ID corresponding to the H5F_t struct */
+ if (H5I_find_id(f, H5I_FILE, &file_id) < 0 || H5I_INVALID_HID == file_id)
+ HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "invalid atom")
+
+ /* Get the number of references outstanding for this file ID */
+ if ((nref = H5I_get_ref(file_id, FALSE)) < 0)
+ HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "can't get ID ref count")
+ if (nref == 1)
+ if (H5F__flush(f) < 0)
+ HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache")
+ } /* end if */
+
+ /* Close the file */
+ if (H5F__close(f) < 0)
+ HGOTO_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "can't close file")
+ }
done:
FUNC_LEAVE_NOAPI(ret_value)