Properly clean up cache when failing to load an object header (#4477)

* Properly clean up cache when failing to load an object header

* Don't check message type a second time in H5G__open_oid if the first attempt returns error

* Add more asserts to H5O__assert() to avoid segfaults

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

src/H5Gint.c

@@ -506,7 +506,8 @@ static herr_t
 H5G__open_oid(H5G_t *grp)
 {
     bool   obj_opened = false;
-    herr_t ret_value  = SUCCEED;
+    htri_t msg_exists;
+    herr_t ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
@@ -523,8 +524,14 @@ H5G__open_oid(H5G_t *grp)
     obj_opened = true;
 
     /* Check if this object has the right message(s) to be treated as a group */
-    if ((H5O_msg_exists(&(grp->oloc), H5O_STAB_ID) <= 0) && (H5O_msg_exists(&(grp->oloc), H5O_LINFO_ID) <= 0))
-        HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, FAIL, "not a group");
+    if ((msg_exists = H5O_msg_exists(&(grp->oloc), H5O_STAB_ID)) < 0)
+        HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "can't check if symbol table message exists");
+    if (!msg_exists) {
+        if ((msg_exists = H5O_msg_exists(&(grp->oloc), H5O_LINFO_ID)) < 0)
+            HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "can't check if link info message exists");
+        if (!msg_exists)
+            HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, FAIL, "not a group");
+    }
 
 done:
     if (ret_value < 0) {

src/H5Odbg.c

@@ -87,6 +87,10 @@ H5O__assert(const H5O_t *oh)
 
     FUNC_ENTER_PACKAGE_NOERR
 
+    assert(oh);
+    assert(oh->chunk || oh->nchunks == 0);
+    assert(oh->mesg || oh->nmesgs == 0);
+
     /* Initialize the tracking variables */
     hdr_size   = 0;
     meta_space = (size_t)H5O_SIZEOF_HDR(oh) + (size_t)(H5O_SIZEOF_CHKHDR_OH(oh) * (oh->nchunks - 1));
@@ -140,6 +144,8 @@ H5O__assert(const H5O_t *oh)
         uint8_t H5_ATTR_NDEBUG_UNUSED *curr_hdr;      /* Start of current message header */
         size_t                         curr_tot_size; /* Total size of current message (including header) */
 
+        assert(curr_msg->type);
+
         curr_hdr      = curr_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh);
         curr_tot_size = curr_msg->raw_size + (size_t)H5O_SIZEOF_MSGHDR_OH(oh);
 

src/H5Oint.c

@@ -1113,7 +1113,9 @@ H5O_protect(const H5O_loc_t *loc, unsigned prot_flags, bool pin_all_chunks)
         if (cont_msg_info.msgs)
             cont_msg_info.msgs = (H5O_cont_t *)H5FL_SEQ_FREE(H5O_cont_t, cont_msg_info.msgs);
 
-        if (H5O_unprotect(loc, oh, H5AC__NO_FLAGS_SET) < 0)
+        /* Unprotect the ohdr and delete it from cache since if we failed to load it it's in an inconsistent
+         * state */
+        if (H5O_unprotect(loc, oh, H5AC__DELETED_FLAG) < 0)
             HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, NULL, "unable to release object header");
     }
 
@@ -1234,10 +1236,21 @@ H5O_unprotect(const H5O_loc_t *loc, H5O_t *oh, unsigned oh_flags)
             } /* end if */
         }     /* end for */
 
-        /* Reet the flag from the unprotect */
+        /* Reset the flag from the unprotect */
         oh->chunks_pinned = false;
     } /* end if */
 
+    /* Remove the other chunks if we're removing the ohdr (due to a failure) */
+    if (oh_flags & H5AC__DELETED_FLAG) {
+        unsigned u; /* Local index variable */
+
+        /* Iterate over chunks > 0 */
+        for (u = 1; u < oh->nchunks; u++)
+            /* Expunge chunk proxy from cache */
+            if (H5AC_expunge_entry(loc->file, H5AC_OHDR_CHK, oh->chunk[u].addr, H5AC__NO_FLAGS_SET) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTUNPIN, FAIL, "unable to expunge object header chunk");
+    } /* end if */
+
     /* Unprotect the object header */
     if (H5AC_unprotect(loc->file, H5AC_OHDR, oh->chunk[0].addr, oh, oh_flags) < 0)
         HGOTO_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to release object header");