Avoid leaving a v1 B-tree used as a chunk index in a bad state

that makes assertions fail.

Add an optional `close` method to the `H5D_chunk_ops_t`, and use that to
release "holds" on metadata cache (MDC) entries.

For extensible arrays and v2 B-trees, use the existing `dest`(roy)
method to implement `close`.  For v1 B-trees and other chunk indices,
don't provide `close`: we cannot safely close the v1 B-tree index, and
the other indices don't have a meaningful presence in the MDC.

Revert my first attempt at making v1 B-tree chunk indices closeable
with `dest`.

Put my comment about the stopgap fix for VFD SWMR at the right place
in src/H5Dchunk.c.

8 files changed, 45 insertions, 33 deletions

diff --git a/src/H5Dbtree.c b/src/H5Dbtree.c
index 996c89e..7741e99 100644
--- a/src/H5Dbtree.c
+++ b/src/H5Dbtree.c
@@ -166,7 +166,8 @@ const H5D_chunk_ops_t H5D_COPS_BTREE[1] = {{
     H5D__btree_idx_size,                /* size */
     H5D__btree_idx_reset,               /* reset */
     H5D__btree_idx_dump,                /* dump */
-    H5D__btree_idx_dest                 /* destroy */
+    H5D__btree_idx_dest,                /* destroy */
+    NULL                                /* close */
 }};
 
 
@@ -1234,8 +1235,8 @@ H5D__btree_idx_delete(const H5D_chk_idx_info_t *idx_info)
 
         /* Release the shared B-tree page */
         if(NULL == tmp_storage.u.btree.shared)
-            ;   /* do nothing */
-        else if(H5UC_DEC(tmp_storage.u.btree.shared) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "ref-counted page nil")
+        if(H5UC_DEC(tmp_storage.u.btree.shared) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to decrement ref-counted page")
     } /* end if */
 
@@ -1454,12 +1455,10 @@ H5D__btree_idx_dest(const H5D_chk_idx_info_t *idx_info)
 
     /* Free the raw B-tree node buffer */
     if(NULL == idx_info->storage->u.btree.shared)
-        HGOTO_DONE(SUCCEED);
+        HGOTO_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "ref-counted page nil")
     if(H5UC_DEC(idx_info->storage->u.btree.shared) < 0)
 	HGOTO_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "unable to decrement ref-counted page")
 
-    idx_info->storage->u.btree.shared = NULL;
-
 done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5D__btree_idx_dest() */
@@ -1519,8 +1518,9 @@ done:
         /* Free the raw B-tree node buffer */
         if(NULL == storage.u.btree.shared)
             HDONE_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "ref-counted shared info nil")
-        else if(H5UC_DEC(storage.u.btree.shared) < 0)
-            HDONE_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "unable to decrement ref-counted shared info")
+        else
+            if(H5UC_DEC(storage.u.btree.shared) < 0)
+                HDONE_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "unable to decrement ref-counted shared info")
     } /* end if */
 
     FUNC_LEAVE_NOAPI(ret_value)
diff --git a/src/H5Dbtree2.c b/src/H5Dbtree2.c
index ae98654..509a67b 100644
--- a/src/H5Dbtree2.c
+++ b/src/H5Dbtree2.c
@@ -161,7 +161,8 @@ const H5D_chunk_ops_t H5D_COPS_BT2[1] = {{
     H5D__bt2_idx_size,                  /* size */
     H5D__bt2_idx_reset,                 /* reset */
     H5D__bt2_idx_dump,                  /* dump */
-    H5D__bt2_idx_dest                   /* destroy */
+    H5D__bt2_idx_dest,                  /* destroy */
+    H5D__bt2_idx_dest                   /* close (same as destroy) */
 }};
 
 
diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c
index 556f61b..a1bbb24 100644
--- a/src/H5Dchunk.c
+++ b/src/H5Dchunk.c
@@ -266,7 +266,7 @@ static herr_t H5D__chunk_flush(H5D_t *dset);
 static herr_t H5D__chunk_io_term(const H5D_chunk_map_t *fm);
 static herr_t H5D__chunk_dest(H5D_t *dset);
 
-static herr_t H5D__chunk_index_close(H5D_t *);
+static herr_t H5D__chunk_index_close(const H5D_t *, bool);
 
 /* Chunk query operation callbacks */
 static int H5D__get_num_chunks_cb(const H5D_chunk_rec_t *chunk_rec, void *_udata);
@@ -2627,7 +2627,16 @@ H5D__chunk_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info,
         chunk_node = H5D_CHUNK_GET_NEXT_NODE(fm, chunk_node);
     } /* end while */
 
-    if(H5D__chunk_index_close(io_info->dset) < 0)
+    /* Stopgap fix for VFD SWMR: close the chunk index so that
+     * pinned/tagged entries in the metadata cache (MDC) are released.
+     *
+     * Extensible chunked datasets use extensible arrays or btrees as
+     * chunk indices.  Open chunk indices leave pinned/tagged entries
+     * in the MDC, and VFD SWMR cannot (yet) evict or refresh those
+     * entries.  After we write refresh routines for those entries, this
+     * stopgap fix can go away.
+     */
+    if(H5D__chunk_index_close(io_info->dset, false) < 0)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL,
             "unable to close chunk index")
 
@@ -2888,31 +2897,37 @@ done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5D__chunk_io_term() */
 
-/* Close the given dataset's chunk index.
+/* Close the given dataset's chunk index, or destroy it if `destroy`
+ * is true.  A closed index merely releases holds on metadata cache
+ * entries; the index can be reopened.  Once a dataset's index is
+ * destroyed, however, the dataset must not try to use the index, again.
  *
  * A useful side-effect of closing the chunk index is the release
  * pinned/tagged metadata cache entries connected with the index.
  */
 static herr_t
-H5D__chunk_index_close(H5D_t *dset)
+H5D__chunk_index_close(const H5D_t *dset, bool destroy)
 {
     H5D_chk_idx_info_t idx_info;
     H5O_storage_chunk_t *sc = &(dset->shared->layout.storage.u.chunk);
     herr_t      ret_value = SUCCEED;       /* Return value */
+    H5D_chunk_close_func_t fn;
 
     FUNC_ENTER_STATIC
 
     H5D_CHUNK_STORAGE_INDEX_CHK(sc);
 
-    /* Compose chunked index info struct */
     idx_info.f = dset->oloc.file;
     idx_info.pline = &dset->shared->dcpl_cache.pline;
     idx_info.layout = &dset->shared->layout.u.chunk;
     idx_info.storage = sc;
 
-    /* Free any index structures */
-    if(sc->ops->dest && (sc->ops->dest)(&idx_info) < 0)
-        HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to release chunk index info")
+    fn = destroy ? sc->ops->dest : sc->ops->close;
+
+    if (fn != NULL && (*fn)(&idx_info) < 0) {
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL,
+            "unable to release chunk index info")
+    }
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
@@ -2960,16 +2975,7 @@ H5D__chunk_dest(H5D_t *dset)
         rdcc->slot = H5FL_SEQ_FREE(H5D_rdcc_ent_ptr_t, rdcc->slot);
     HDmemset(rdcc, 0, sizeof(H5D_rdcc_t));
 
-    /* Stopgap fix for VFD SWMR: close the chunk index so that
-     * pinned/tagged entries in the metadata cache (MDC) are released.
-     *
-     * Extensible chunked datasets use extensible arrays or btrees as
-     * chunk indices.  Open chunk indices leave pinned/tagged entries
-     * in the MDC, and VFD SWMR cannot (yet) evict or refresh those
-     * entries.  After we write refresh routines for those entries, this
-     * stopgap fix can go away.
-     */
-    if (H5D__chunk_index_close(dset) < 0) {
+    if (H5D__chunk_index_close(dset, true) < 0) {
         HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL,
             "unable to close chunk index")
     }
diff --git a/src/H5Dearray.c b/src/H5Dearray.c
index eaa8c46..3a9dcc7 100644
--- a/src/H5Dearray.c
+++ b/src/H5Dearray.c
@@ -165,7 +165,8 @@ const H5D_chunk_ops_t H5D_COPS_EARRAY[1] = {{
     H5D__earray_idx_size,               /* size */
     H5D__earray_idx_reset,              /* reset */
     H5D__earray_idx_dump,               /* dump */
-    H5D__earray_idx_dest                /* destroy */
+    H5D__earray_idx_dest,               /* destroy */
+    H5D__earray_idx_dest                /* close (same as destroy) */
 }};
 
 
diff --git a/src/H5Dfarray.c b/src/H5Dfarray.c
index cbaa52d..bcdbc92 100644
--- a/src/H5Dfarray.c
+++ b/src/H5Dfarray.c
@@ -161,7 +161,8 @@ const H5D_chunk_ops_t H5D_COPS_FARRAY[1] = {{
     H5D__farray_idx_size,               /* size */
     H5D__farray_idx_reset,              /* reset */
     H5D__farray_idx_dump,               /* dump */
-    H5D__farray_idx_dest                /* destroy */
+    H5D__farray_idx_dest,               /* destroy */
+    NULL                                /* close */
 }};
 
 
diff --git a/src/H5Dnone.c b/src/H5Dnone.c
index 5ba3f5b..654775e 100644
--- a/src/H5Dnone.c
+++ b/src/H5Dnone.c
@@ -95,7 +95,8 @@ const H5D_chunk_ops_t H5D_COPS_NONE[1] = {{
     H5D__none_idx_size,			/* size */
     H5D__none_idx_reset,		/* reset */
     H5D__none_idx_dump,			/* dump */
-    NULL				/* dest */
+    NULL,				/* dest */
+    NULL				/* close */
 }};
 
 
diff --git a/src/H5Dpkg.h b/src/H5Dpkg.h
index c46e38b6..e74501d 100644
--- a/src/H5Dpkg.h
+++ b/src/H5Dpkg.h
@@ -308,7 +308,7 @@ typedef herr_t (*H5D_chunk_size_func_t)(const H5D_chk_idx_info_t *idx_info,
 typedef herr_t (*H5D_chunk_reset_func_t)(H5O_storage_chunk_t *storage, hbool_t reset_addr);
 typedef herr_t (*H5D_chunk_dump_func_t)(const H5O_storage_chunk_t *storage,
     FILE *stream);
-typedef herr_t (*H5D_chunk_dest_func_t)(const H5D_chk_idx_info_t *idx_info);
+typedef herr_t (*H5D_chunk_close_func_t)(const H5D_chk_idx_info_t *idx_info);
 
 /* Typedef for grouping chunk I/O routines */
 typedef struct H5D_chunk_ops_t {
@@ -327,7 +327,8 @@ typedef struct H5D_chunk_ops_t {
     H5D_chunk_size_func_t size;             /* Routine to get size of indexing information */
     H5D_chunk_reset_func_t reset;           /* Routine to reset indexing information */
     H5D_chunk_dump_func_t dump;             /* Routine to dump indexing information */
-    H5D_chunk_dest_func_t dest;             /* Routine to destroy indexing information in memory */
+    H5D_chunk_close_func_t dest;             /* Routine to destroy indexing information in memory */
+    H5D_chunk_close_func_t close;             /* Routine to destroy indexing information in memory */
 } H5D_chunk_ops_t;
 
 /* Structure holding information about a chunk's selection for mapping */
diff --git a/src/H5Dsingle.c b/src/H5Dsingle.c
index 33274bb..3fa9bc2 100644
--- a/src/H5Dsingle.c
+++ b/src/H5Dsingle.c
@@ -97,7 +97,8 @@ const H5D_chunk_ops_t H5D_COPS_SINGLE[1] = {{
     H5D__single_idx_size,		/* size */
     H5D__single_idx_reset,		/* reset */
     H5D__single_idx_dump,		/* dump */
-    NULL				/* destroy */
+    NULL,				/* destroy */
+    NULL				/* close */
 }};