[svn-r23135] Purpose: Fix bug in SWMR object header code

Descriptions:
When removing object header messages, it is possible for object header
continuation messages to move to a different chunk.  When this happens, flush
dependencies need to be updated to reflect the new structure.  This change adds
code to update the flush dependencies, and a test for this.

Also fixed a bug where the flush dependency no the object header proxy was not
being destroyed when an object header chunk was deleted.

Tested: ummon

4 files changed, 117 insertions, 24 deletions

diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c
index 0ff52f1..c889abb 100644
--- a/src/H5Oalloc.c
+++ b/src/H5Oalloc.c
@@ -920,7 +920,7 @@ H5O_alloc_new_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh, size_t size, size_t *new
         oh->chunk = x;
     } /* end if */
 
-    chunkno = oh->nchunks++;
+    chunkno = (unsigned)oh->nchunks++;
     oh->chunk[chunkno].addr = new_chunk_addr;
     oh->chunk[chunkno].size = size;
     oh->chunk[chunkno].gap = 0;
@@ -965,6 +965,8 @@ H5O_alloc_new_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh, size_t size, size_t *new
                     oh->nmesgs--;
                 } /* end if */
                 else {
+                    HDassert(curr_msg->type->id != H5O_CONT_ID);
+
                     /* Copy the raw data */
                     HDmemcpy(p, curr_msg->raw - (size_t)H5O_SIZEOF_MSGHDR_OH(oh),
                         curr_msg->raw_size + (size_t)H5O_SIZEOF_MSGHDR_OH(oh));
@@ -1454,6 +1456,7 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh)
 {
     H5O_chunk_proxy_t *null_chk_proxy = NULL;  /* Chunk that null message is in */
     H5O_chunk_proxy_t *curr_chk_proxy = NULL;  /* Chunk that message is in */
+    H5O_chunk_proxy_t *cont_targ_chk_proxy = NULL; /* Chunk that continuation message points to */
     hbool_t null_chk_dirtied = FALSE;  /* Flags for unprotecting null chunk */
     hbool_t curr_chk_dirtied = FALSE;  /* Flags for unprotecting curr chunk */
     hbool_t packed_msg;                 /* Flag to indicate that messages were packed */
@@ -1543,13 +1546,13 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh)
                 if(H5O_CONT_ID == curr_msg->type->id) {
                     htri_t status;      /* Status from moving messages */
 
-		    if((status = H5O_move_cont(f, dxpl_id, oh, u)) < 0)
-			HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "Error in moving messages into cont message")
-		    else if(status > 0) { /* Message(s) got moved into "continuation" message */
+                    if((status = H5O_move_cont(f, dxpl_id, oh, u)) < 0)
+                        HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "Error in moving messages into cont message")
+                    else if(status > 0) { /* Message(s) got moved into "continuation" message */
                         packed_msg = TRUE;
-			break;
-		    } /* end else-if */
-		} /* end if */
+                        break;
+                    } /* end else-if */
+                } /* end if */
 
                 /* Don't let locked messages be moved into earlier chunk */
                 if(!curr_msg->locked) {
@@ -1570,6 +1573,46 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh)
                             if(NULL == (curr_chk_proxy = H5O_chunk_protect(f, dxpl_id, oh, curr_msg->chunkno)))
                                 HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, FAIL, "unable to load object header chunk")
 
+                            /* If the message being moved is a continuation
+                             * message and we are doing SWMR writes, we must
+                             * update the flush dependencies */
+                            if((H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)
+                                    && (H5O_CONT_ID == curr_msg->type->id)) {
+                                void *null_chk_mdc_obj = NULL; /* The metadata cache object for the null_msg chunk */
+
+                                /* Point to the metadata cache object for the
+                                 * null message chunk, oh if in chunk 0 or the
+                                 * proxy otherwise */
+                                null_chk_mdc_obj = (null_msg->chunkno == 0
+                                        ? (void *)oh
+                                        : (void *)null_chk_proxy);
+
+                                /* The other chunks involved should never be
+                                 * chunk 0 */
+                                HDassert(curr_msg->chunkno > 0);
+                                HDassert(((H5O_cont_t *)(curr_msg->native))->chunkno > 0);
+
+                                /* Protect continuation message target chunk */
+                                if(NULL == (cont_targ_chk_proxy = H5O_chunk_protect(f, dxpl_id, oh, ((H5O_cont_t *)(curr_msg->native))->chunkno)))
+                                    HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, FAIL, "unable to load object header chunk")
+
+                                /* Remove flush dependency on old continuation
+                                 * message chunk */
+                                if(H5AC_destroy_flush_dependency(curr_chk_proxy, cont_targ_chk_proxy) < 0)
+                                    HGOTO_ERROR(H5E_OHDR, H5E_CANTUNDEPEND, FAIL, "unable to destroy flush dependency")
+
+                                /* Create flush dependency on new continuation
+                                 * message chunk */
+                                if(H5AC_create_flush_dependency(null_chk_mdc_obj, cont_targ_chk_proxy) < 0)
+                                    HGOTO_ERROR(H5E_OHDR, H5E_CANTDEPEND, FAIL, "unable to create flush dependency")
+
+                                /* Unprotect continuation message target chunk
+                                 */
+                                if(H5O_chunk_unprotect(f, dxpl_id, cont_targ_chk_proxy, FALSE) < 0)
+                                    HGOTO_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect object header chunk")
+                                cont_targ_chk_proxy = NULL;
+                            } /* end if */
+
                             /* Copy raw data for non-null message to new chunk */
                             HDmemcpy(null_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh), curr_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh), curr_msg->raw_size + (size_t)H5O_SIZEOF_MSGHDR_OH(oh));
 
@@ -1714,10 +1757,16 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh)
     ret_value = (htri_t)did_packing;
 
 done:
-    if(null_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, null_chk_proxy, null_chk_dirtied) < 0)
-        HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect null object header chunk")
-    if(curr_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, curr_chk_proxy, curr_chk_dirtied) < 0)
-        HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect current object header chunk")
+    if(ret_value < 0) {
+        if(null_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, null_chk_proxy, null_chk_dirtied) < 0)
+            HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect null object header chunk")
+        if(curr_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, curr_chk_proxy, curr_chk_dirtied) < 0)
+            HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect current object header chunk")
+        if(cont_targ_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, cont_targ_chk_proxy, FALSE) < 0)
+            HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect continuation message target object header chunk")
+    } /* end if */
+    else
+        HDassert(!null_chk_proxy && !curr_chk_proxy && !cont_targ_chk_proxy);
 
     FUNC_LEAVE_NOAPI(ret_value)
 } /* H5O_move_msgs_forward() */
diff --git a/src/H5Ochunk.c b/src/H5Ochunk.c
index af4233e..8b380b2 100644
--- a/src/H5Ochunk.c
+++ b/src/H5Ochunk.c
@@ -402,8 +402,15 @@ H5O_chunk_delete(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned idx)
     HDassert(chk_proxy->oh == oh);
     HDassert(chk_proxy->chunkno == idx);
 
-    /* Only free file space if not doing SWMR writes */
-    if(!(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE))
+    /* Update flush dependencies if doing SWMR writes */
+    if(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE) {
+        /* Remove flush dependency on object header proxy, if proxy exists */
+        if(oh->proxy_present)
+            if(H5O_proxy_undepend(f, dxpl_id, oh, chk_proxy) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTUNDEPEND, FAIL, "can't destroy flush dependency on object header proxy")
+    } /* end if */
+    else
+        /* Only free file space if not doing SWMR writes */
         cache_flags |= H5AC__DIRTIED_FLAG | H5AC__FREE_FILE_SPACE_FLAG;
 
     /* Release the chunk proxy from the cache, marking it deleted */
diff --git a/test/swmr_generator.c b/test/swmr_generator.c
index f904378..f0fcee6 100644
--- a/test/swmr_generator.c
+++ b/test/swmr_generator.c
@@ -42,8 +42,9 @@
 /* Local Prototypes */
 /********************/
 
-static int gen_skeleton(const char *filename, unsigned verbose, int comp_level,
-    const char *index_type, unsigned random_seed);
+static int gen_skeleton(const char *filename, unsigned verbose,
+    unsigned swmr_write, int comp_level, const char *index_type,
+    unsigned random_seed);
 static void usage(void);
 
 
@@ -76,8 +77,8 @@ static void usage(void);
  *-------------------------------------------------------------------------
  */
 static int
-gen_skeleton(const char *filename, unsigned verbose, int comp_level,
-    const char *index_type, unsigned random_seed)
+gen_skeleton(const char *filename, unsigned verbose, unsigned swmr_write,
+    int comp_level, const char *index_type, unsigned random_seed)
 {
     hid_t fid;          /* File ID for new HDF5 file */
     hid_t fcpl;         /* File creation property list */
@@ -145,7 +146,7 @@ gen_skeleton(const char *filename, unsigned verbose, int comp_level,
         fprintf(stderr, "Creating file\n");
 
     /* Create the file */
-    if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, fcpl, fapl)) < 0)
+    if((fid = H5Fcreate(filename, H5F_ACC_TRUNC | (swmr_write ? H5F_ACC_SWMR_WRITE : 0), fcpl, fapl)) < 0)
         return -1;
 
     /* Close file creation property list */
@@ -234,8 +235,8 @@ usage(void)
     printf("\n");
     printf("Usage error!\n");
     printf("\n");
-    printf("Usage: swmr_generator [-q] [-c <deflate compression level>] [-i <index type>]\n");
-    printf("    [-r <random seed>]\n");
+    printf("Usage: swmr_generator [-q] [-s] [-c <deflate compression level>]\n");
+    printf("    [-i <index type>] [-r <random seed>]\n");
     printf("\n");
     printf("NOTE: The random seed option is only used by the sparse test.  Other\n");
     printf("      tests specify the random seed as a reader/writer option.\n");
@@ -244,8 +245,9 @@ usage(void)
     printf("\n");
     printf("<index type> should be b1, b2, fa, or ea (fa not yet implemented)\n");
     printf("\n");
-    printf("Defaults to verbose (no '-q' given), no compression ('-c -1'), v1 b-tree\n");
-    printf("indexing (-i b1), and will generate a random seed (no -r given).\n");
+    printf("Defaults to verbose (no '-q' given), no SWMR_WRITE mode (no '-s' given) no\n");
+    printf("compression ('-c -1'), v1 b-tree indexing (-i b1), and will generate a random\n");
+    printf("seed (no -r given).\n");
     printf("\n");
     exit(1);
 } /* end usage() */
@@ -254,6 +256,7 @@ int main(int argc, const char *argv[])
 {
     int comp_level = -1;            /* Compression level (-1 is no compression) */
     unsigned verbose = 1;           /* Whether to emit some informational messages */
+    unsigned swmr_write = 0;        /* Whether to create file with SWMR_WRITE access */
     const char *index_type = "b1";  /* Chunk index type */
     unsigned use_seed = 0;          /* Set to 1 if a seed was set on the command line */
     unsigned random_seed = 0;       /* Random # seed */
@@ -301,6 +304,12 @@ int main(int argc, const char *argv[])
                         u++;
                         break;
 
+                    /* Run with SWMR_WRITE */
+                    case 's':
+                        swmr_write = 1;
+                        u++;
+                        break;
+
                     default:
                         usage();
                         break;
@@ -312,8 +321,9 @@ int main(int argc, const char *argv[])
     /* Emit informational message */
     if(verbose) {
         fprintf(stderr, "Parameters:\n");
+        fprintf(stderr, "\tswmr writes %s\n", swmr_write ? "on" : "off");
         fprintf(stderr, "\tcompression level = %d\n", comp_level);
-        fprintf(stderr, "\tindex_type = %s\n", index_type);
+        fprintf(stderr, "\tindex type = %s\n", index_type);
     } /* end if */
     
     /* Set the random seed */
@@ -331,7 +341,7 @@ int main(int argc, const char *argv[])
         fprintf(stderr, "Generating skeleton file: %s\n", FILENAME);
 
     /* Generate file skeleton */
-    if(gen_skeleton(FILENAME, verbose, comp_level, index_type, random_seed) < 0) {
+    if(gen_skeleton(FILENAME, verbose, swmr_write, comp_level, index_type, random_seed) < 0) {
         fprintf(stderr, "Error generating skeleton file!\n");
         exit(1);
     } /* end if */
diff --git a/test/testswmr.sh b/test/testswmr.sh
index 43f9f39..b178af0 100755
--- a/test/testswmr.sh
+++ b/test/testswmr.sh
@@ -76,6 +76,33 @@ do
     do
         echo
         echo "###############################################################################"
+        echo "## Generator test"
+        echo "###############################################################################"
+        # Launch the Generator without SWMR_WRITE
+        echo launch the swmr_generator
+        ./swmr_generator $compress $index_type
+        if test $? -ne 0; then
+            echo generator had error
+            nerrors=`expr $nerrors + 1`
+        fi
+
+        # Launch the Generator with SWMR_WRITE
+        echo launch the swmr_generator with SWMR_WRITE
+        ./swmr_generator -s $compress $index_type
+        if test $? -ne 0; then
+            echo generator had error
+            nerrors=`expr $nerrors + 1`
+        fi
+
+        # Check for error and exit if one occured
+        $DPRINT nerrors=$nerrors
+        if test $nerrors -ne 0 ; then
+            echo "SWMR tests failed with $nerrors errors."
+            exit 1
+        fi
+
+        echo
+        echo "###############################################################################"
         echo "## Writer test - test expanding the dataset"
         echo "###############################################################################"