Fixed a memory issue where unfreed shared attribute dataspace memory

tripped an assert in our memory sanity checks.

Fixes HDFFV-10774.

4 files changed, 25 insertions, 25 deletions

diff --git a/src/H5Adense.c b/src/H5Adense.c
index 81e0dc5..ceac7d9 100644
--- a/src/H5Adense.c
+++ b/src/H5Adense.c
@@ -325,14 +325,10 @@ H5A__dense_fnd_cb(const H5A_t *attr, hbool_t *took_ownership, void *_user_attr)
      */
     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 */
+        /* Free any dynamically allocated items */
+        if(H5A__free(old_attr) < 0)
+            HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release attribute info")
 
         old_attr = H5FL_FREE(H5A_t, old_attr);
      } /* end if */
diff --git a/src/H5Aint.c b/src/H5Aint.c
index 6162401..2240657 100644
--- a/src/H5Aint.c
+++ b/src/H5Aint.c
@@ -1113,24 +1113,32 @@ H5A__free(H5A_t *attr)
 
     HDassert(attr);
 
-    /* Free dynamically allocated items */
+    if(!attr->shared)
+        HGOTO_DONE(SUCCEED)
+
+    /* Free dynamically allocated items.
+     * When possible, keep trying to shut things down (via HDONE_ERROR).
+     */
     if(attr->shared->name) {
         H5MM_xfree(attr->shared->name);
         attr->shared->name = NULL;
     }
     if(attr->shared->dt) {
         if(H5T_close_real(attr->shared->dt) < 0)
-            HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release datatype info")
+            HDONE_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release datatype info")
         attr->shared->dt = NULL;
     }
     if(attr->shared->ds) {
         if(H5S_close(attr->shared->ds) < 0)
-            HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release dataspace info")
+            HDONE_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release dataspace info")
         attr->shared->ds = NULL;
     }
     if(attr->shared->data)
         attr->shared->data = H5FL_BLK_FREE(attr_buf, attr->shared->data);
 
+    /* Destroy shared attribute struct */
+    attr->shared = H5FL_FREE(H5A_shared_t, attr->shared);
+
 done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5A__free() */
@@ -1199,9 +1207,6 @@ H5A__close(H5A_t *attr)
         /* Free dynamically allocated items */
         if(H5A__free(attr) < 0)
             HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "can't release attribute info")
-
-        /* Destroy shared attribute struct */
-        attr->shared = H5FL_FREE(H5A_shared_t, attr->shared);
     } /* end if */
     else {
         /* There are other references to the shared part of the attribute.
diff --git a/src/H5Oattr.c b/src/H5Oattr.c
index 0a7c4bf..653c23a 100644
--- a/src/H5Oattr.c
+++ b/src/H5Oattr.c
@@ -200,7 +200,7 @@ H5O_attr_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags,
      * What's actually shared, though, is only the extent.
      */
     if(NULL == (attr->shared->ds = H5FL_CALLOC(H5S_t)))
-	HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
+        HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
 
     /* Decode attribute's dataspace extent */
     if((extent = (H5S_extent_t *)(H5O_MSG_SDSPACE->decode)(f, open_oh,
@@ -253,14 +253,9 @@ H5O_attr_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags,
 done:
     if(NULL == ret_value)
         if(attr) {
-            if(attr->shared) {
-                /* Free any dynamically allocated items */
-                if(H5A__free(attr) < 0)
-                    HDONE_ERROR(H5E_ATTR, H5E_CANTRELEASE, NULL, "can't release attribute info")
-
-                /* Destroy shared attribute struct */
-                attr->shared = H5FL_FREE(H5A_shared_t, attr->shared);
-            } /* end if */
+            /* Free any dynamically allocated items */
+            if(H5A__free(attr) < 0)
+                HDONE_ERROR(H5E_ATTR, H5E_CANTRELEASE, NULL, "can't release attribute info")
 
             attr = H5FL_FREE(H5A_t, attr);
         } /* end if */
diff --git a/src/H5S.c b/src/H5S.c
index 3926b5f..e50985f 100644
--- a/src/H5S.c
+++ b/src/H5S.c
@@ -448,10 +448,14 @@ H5S_close(H5S_t *ds)
     if(H5S__extent_release(&ds->extent) < 0)
         HGOTO_ERROR(H5E_DATASPACE, H5E_CANTRELEASE, FAIL, "unable to release dataspace extent")
 
-    /* Release the main structure */
-    ds = H5FL_FREE(H5S_t, ds);
-
 done:
+    /* Release the main structure.
+     * Always do this to ensure that we don't leak memory when calling this
+     * function on partially constructed dataspaces (which will fail one or
+     * both of the above calls)
+     */
+    H5FL_FREE(H5S_t, ds);
+
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5S_close() */