Merge pull request #2237 in HDFFV/hdf5 from ~JSOUMAGNE/hdf5:HDFFV-10992-fix to develop

* commit 'a7648879d729c1b75bd32f3a151831e9cbfbe31c':
  Add test for reference shutdown issue
  H5R: set app ref when incrementing ref_count on location held by reference (fix HDFFV-10992)

7 files changed, 123 insertions, 18 deletions

diff --git a/MANIFEST b/MANIFEST
index bf6b460..b1b8f07 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -1184,6 +1184,7 @@
 ./test/ttime.c
 ./test/trefer.c
 ./test/trefer_deprec.c
+./test/trefer_shutdown.c
 ./test/trefstr.c
 ./test/tselect.c
 ./test/tsizeslheap.h5
diff --git a/src/H5Ocopy_ref.c b/src/H5Ocopy_ref.c
index b835f8e..f53eb4a 100644
--- a/src/H5Ocopy_ref.c
+++ b/src/H5Ocopy_ref.c
@@ -383,7 +383,8 @@ H5O__copy_expand_ref_object2(H5O_loc_t *src_oloc, hid_t tid_src, H5T_t *dt_src,
         H5F_addr_encode(dst_oloc->file, &p, dst_oloc->addr);
         if(H5R__set_obj_token(ref, (const H5VL_token_t *)&tmp_token, token_size) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "unable to set object token")
-        if(H5R__set_loc_id(ref, dst_loc_id, TRUE) < 0)
+        /* Do not set app_ref since references are released once the copy is done */
+        if(H5R__set_loc_id(ref, dst_loc_id, TRUE, FALSE) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "unable to set destination loc id")
     } /* end for */
 
diff --git a/src/H5R.c b/src/H5R.c
index f5d119f..3a022eb 100644
--- a/src/H5R.c
+++ b/src/H5R.c
@@ -139,7 +139,7 @@ H5Rcreate_object(hid_t loc_id, const char *name, hid_t oapl_id, H5R_ref_t *ref_p
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create object reference")
 
     /* Attach loc_id to reference and hold reference to it */
-    if(H5R__set_loc_id((H5R_ref_priv_t *)ref_ptr, file_id, TRUE) < 0)
+    if(H5R__set_loc_id((H5R_ref_priv_t *)ref_ptr, file_id, TRUE, TRUE) < 0)
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "unable to attach location id to reference")
 
 done:
@@ -232,7 +232,7 @@ H5Rcreate_region(hid_t loc_id, const char *name, hid_t space_id,
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create region reference")
 
     /* Attach loc_id to reference and hold reference to it */
-    if(H5R__set_loc_id((H5R_ref_priv_t *)ref_ptr, file_id, TRUE) < 0)
+    if(H5R__set_loc_id((H5R_ref_priv_t *)ref_ptr, file_id, TRUE, TRUE) < 0)
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "unable to attach location id to reference")
 
 done:
@@ -321,7 +321,7 @@ H5Rcreate_attr(hid_t loc_id, const char *name, const char *attr_name,
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create attribute reference")
 
     /* Attach loc_id to reference and hold reference to it */
-    if(H5R__set_loc_id((H5R_ref_priv_t *)ref_ptr, file_id, TRUE) < 0)
+    if(H5R__set_loc_id((H5R_ref_priv_t *)ref_ptr, file_id, TRUE, TRUE) < 0)
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "unable to attach location id to reference")
 
 done:
diff --git a/src/H5Rint.c b/src/H5Rint.c
index 46e4c0c..cef1f0a 100644
--- a/src/H5Rint.c
+++ b/src/H5Rint.c
@@ -433,8 +433,15 @@ H5R__destroy(H5R_ref_priv_t *ref)
     } /* end switch */
 
     /* Decrement refcount of attached loc_id */
-    if(ref->type && (ref->loc_id != H5I_INVALID_HID) && (H5I_dec_ref(ref->loc_id) < 0))
-        HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "decrementing location ID failed")
+    if(ref->type && (ref->loc_id != H5I_INVALID_HID)) {
+        if(ref->app_ref) {
+            if(H5I_dec_app_ref(ref->loc_id) < 0)
+                HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "decrementing location ID failed")
+        } else {
+            if(H5I_dec_ref(ref->loc_id) < 0)
+                HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "decrementing location ID failed")
+        }
+    }
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
@@ -451,7 +458,7 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5R__set_loc_id(H5R_ref_priv_t *ref, hid_t id, hbool_t inc_ref)
+H5R__set_loc_id(H5R_ref_priv_t *ref, hid_t id, hbool_t inc_ref, hbool_t app_ref)
 {
     herr_t ret_value = SUCCEED; /* Return value */
 
@@ -460,14 +467,26 @@ H5R__set_loc_id(H5R_ref_priv_t *ref, hid_t id, hbool_t inc_ref)
     HDassert(ref != NULL);
     HDassert(id != H5I_INVALID_HID);
 
-    /* If a location ID was previously assigned, decrement refcount and assign new one */
-    if((ref->loc_id != H5I_INVALID_HID) && (H5I_dec_ref(ref->loc_id) < 0))
-        HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "decrementing location ID failed")
+    /* If a location ID was previously assigned, decrement refcount and
+     * assign new one */
+    if((ref->loc_id != H5I_INVALID_HID)) {
+        if(ref->app_ref) {
+            if(H5I_dec_app_ref(ref->loc_id) < 0)
+                HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "decrementing location ID failed")
+        } else {
+            if(H5I_dec_ref(ref->loc_id) < 0)
+                HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "decrementing location ID failed")
+        }
+    }
     ref->loc_id = id;
 
-    /* Prevent location ID from being freed until reference is destroyed */
-    if(inc_ref && H5I_inc_ref(ref->loc_id, FALSE) < 0)
+    /* Prevent location ID from being freed until reference is destroyed,
+     * set app_ref if necessary as references are exposed to users and are
+     * expected to be destroyed, this allows the loc_id to be cleanly released
+     * on shutdown if users fail to call H5Rdestroy(). */
+    if(inc_ref && H5I_inc_ref(ref->loc_id, app_ref) < 0)
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTINC, FAIL, "incrementing location ID failed")
+    ref->app_ref = app_ref;
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
@@ -559,7 +578,7 @@ H5R__reopen_file(H5R_ref_priv_t *ref, hid_t fapl_id)
             HGOTO_ERROR(H5E_REFERENCE, H5E_CANTINIT, H5I_INVALID_HID, "unable to make file 'post open' callback")
 
     /* Attach loc_id to reference */
-    if(H5R__set_loc_id((H5R_ref_priv_t *)ref, ret_value, FALSE) < 0)
+    if(H5R__set_loc_id((H5R_ref_priv_t *)ref, ret_value, FALSE, TRUE) < 0)
         HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, H5I_INVALID_HID, "unable to attach location id to reference")
 
 done:
@@ -711,7 +730,7 @@ H5R__copy(const H5R_ref_priv_t *src_ref, H5R_ref_priv_t *dst_ref)
         dst_ref->ref.obj.filename = NULL;
 
         /* Set location ID and hold reference to it */
-        if(H5R__set_loc_id(dst_ref, src_ref->loc_id, TRUE) < 0)
+        if(H5R__set_loc_id(dst_ref, src_ref->loc_id, TRUE, TRUE) < 0)
             HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "cannot set reference location ID")
     }
 
diff --git a/src/H5Rpkg.h b/src/H5Rpkg.h
index 7471487..4dbc656 100644
--- a/src/H5Rpkg.h
+++ b/src/H5Rpkg.h
@@ -79,7 +79,7 @@ typedef struct H5R_ref_priv_t {
     uint32_t encode_size;       /* Cached encoding size             */
     int8_t   type;              /* Reference type                   */
     uint8_t  token_size;        /* Cached token size                */
-    char unused[18];            /* Unused                           */
+    hbool_t  app_ref;           /* App ref on loc_id                */
 } H5R_ref_priv_t;
 
 /*****************************/
@@ -95,7 +95,7 @@ H5_DLL herr_t   H5R__create_region(const H5VL_token_t *obj_token, size_t token_s
 H5_DLL herr_t   H5R__create_attr(const H5VL_token_t *obj_token, size_t token_size, const char *attr_name, H5R_ref_priv_t *ref);
 H5_DLL herr_t   H5R__destroy(H5R_ref_priv_t *ref);
 
-H5_DLL herr_t   H5R__set_loc_id(H5R_ref_priv_t *ref, hid_t id, hbool_t inc_ref);
+H5_DLL herr_t   H5R__set_loc_id(H5R_ref_priv_t *ref, hid_t id, hbool_t inc_ref, hbool_t app_ref);
 H5_DLL hid_t    H5R__get_loc_id(const H5R_ref_priv_t *ref);
 H5_DLL hid_t    H5R__reopen_file(H5R_ref_priv_t *ref, hid_t fapl_id);
 
diff --git a/src/H5Tref.c b/src/H5Tref.c
index f31fecf..8288912 100644
--- a/src/H5Tref.c
+++ b/src/H5Tref.c
@@ -622,8 +622,9 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
         if((file_id = H5F_get_file_id(src_file, H5I_FILE, FALSE)) < 0)
             HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file or file object")
 
-        /* Attach loc ID to reference and hold reference to it */
-        if(H5R__set_loc_id(dst_ref, file_id, TRUE) < 0)
+        /* Attach loc ID to reference and hold reference to it, this is a
+         * user exposed reference so set app_ref to TRUE. */
+        if(H5R__set_loc_id(dst_ref, file_id, TRUE, TRUE) < 0)
             HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "unable to attach location id to reference")
     } /* end if */
 
diff --git a/test/trefer_shutdown.c b/test/trefer_shutdown.c
new file mode 100644
index 0000000..89a44b3
--- /dev/null
+++ b/test/trefer_shutdown.c
@@ -0,0 +1,83 @@
+#include "h5test.h"
+
+int
+main(int argc, char **argv)
+{
+    H5R_ref_t write_ref, read_ref;
+    hid_t     fid;
+    hid_t     did;
+    hid_t     sid;
+    int i;
+
+    if ((fid = H5Fcreate("HDFFV-10992.h5", H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
+        HDfprintf(stderr, "H5Fcreate failed\n");
+        return 1;
+    }
+
+    if ((sid = H5Screate(H5S_SCALAR)) < 0) {
+        HDfprintf(stderr, "H5Screate failed\n");
+        return 1;
+    }
+
+    /* Create a dataset of object references */
+    if ((did = H5Dcreate2(fid, "dset", H5T_STD_REF, sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
+        HDfprintf(stderr, "H5Dcreate failed\n");
+        return 1;
+    }
+
+    for (i = 0; i < 100; i++) {
+        /* Create reference to the root group */
+        if (H5Rcreate_object(fid, "/dset", H5P_DEFAULT, &write_ref) < 0) {
+            HDfprintf(stderr, "H5Rcreate_object failed\n");
+            return 1;
+        }
+    }
+
+    /* Write reference to dataset */
+    if (H5Dwrite(did, H5T_STD_REF, H5S_ALL, H5S_ALL, H5P_DEFAULT, &write_ref) < 0) {
+        HDfprintf(stderr, "H5Dwrite failed\n");
+        return 1;
+    }
+
+    for (i = 0; i < 500; i++) {
+        /* Read reference back into different reference buffer */
+        if (H5Dread(did, H5T_STD_REF, H5S_ALL, H5S_ALL, H5P_DEFAULT, &read_ref) < 0) {
+            HDfprintf(stderr, "H5Dread failed\n");
+            return 1;
+        }
+    }
+
+    /*
+     * "Forget" to call H5Rdestroy on reference objects. If H5Rdestroy
+     * is called at least once on either reference object, or both
+     * objects, the infinite loop goes away. If H5Rdestroy is never 
+     * called, the infinite loop will appear.
+     */
+#if 0
+    if (H5Rdestroy(&write_ref) < 0) {
+        HDfprintf(stderr, "H5Rdestroy on reference write buffer failed\n");
+        return 1;
+    }
+    if (H5Rdestroy(&read_ref) < 0) {
+        HDfprintf(stderr, "H5Rdestroy on reference read buffer failed\n");
+        return 1;
+    }
+#endif
+
+    if (H5Sclose(sid) < 0) {
+        HDfprintf(stderr, "H5Sclose failed\n");
+        return 1;
+    }
+
+    if (H5Dclose(did) < 0) {
+        HDfprintf(stderr, "H5Dclose failed\n");
+        return 1;
+    }
+
+    if (H5Fclose(fid) < 0) {
+        HDfprintf(stderr, "H5Fclose failed\n");
+        return 1;
+    }
+
+    return 0;
+}