From 35bbc743d4cf77d6aa8af2acf5578db02e5129ca Mon Sep 17 00:00:00 2001 From: Vailin Choi Date: Thu, 16 Apr 2009 10:21:01 -0500 Subject: [svn-r16764] To fix a bug in extensible arrays as chunk index: When the dataset is closed in H5D_close(), the pointer to the extensible array struct in the layout message is copied via H5D_flush_real() before H5D_chunk_dest(). This causes an abort from "Assertion `ea->hdr' failed" later when the dataset is re-opened and read. The bug was fixed by adding a flag to the reset function for indexed storage to set/not set the address of the indexed array. H5O_layout_copy() calls H5D_chunk_idx_reset() to reset only the pointer of the array struct for the chunked index storage. --- src/H5Dbtree.c | 11 +++-- src/H5Dchunk.c | 16 ++++++-- src/H5Dearray.c | 11 +++-- src/H5Dpkg.h | 2 +- src/H5Dprivate.h | 2 +- src/H5Olayout.c | 15 ++++++- src/H5Pdcpl.c | 7 +++- test/dsets.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 173 insertions(+), 14 deletions(-) diff --git a/src/H5Dbtree.c b/src/H5Dbtree.c index 7db99d0..fe0349c 100644 --- a/src/H5Dbtree.c +++ b/src/H5Dbtree.c @@ -153,7 +153,7 @@ static herr_t H5D_btree_idx_copy_shutdown(H5O_layout_t *layout_src, H5O_layout_t *layout_dst, hid_t dxpl_id); static herr_t H5D_btree_idx_size(const H5D_chk_idx_info_t *idx_info, hsize_t *size); -static herr_t H5D_btree_idx_reset(H5O_layout_t *layout); +static herr_t H5D_btree_idx_reset(H5O_layout_t *layout, hbool_t reset_addr); static herr_t H5D_btree_idx_dump(const H5D_chk_idx_info_t *idx_info, FILE *stream); static herr_t H5D_btree_idx_dest(const H5D_chk_idx_info_t *idx_info); @@ -1347,17 +1347,22 @@ done: * Programmer: Quincey Koziol * Thursday, January 15, 2009 * + * Modifications: + * Vailin Choi; April 2009 + * Reset address of the chunked storage index if RESET_ADDR is set + * *------------------------------------------------------------------------- */ static herr_t -H5D_btree_idx_reset(H5O_layout_t *layout) +H5D_btree_idx_reset(H5O_layout_t *layout, hbool_t reset_addr) { FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5D_btree_idx_reset) HDassert(layout); /* Reset index info */ - layout->u.chunk.u.btree.addr = HADDR_UNDEF; + if(reset_addr) + layout->u.chunk.u.btree.addr = HADDR_UNDEF; layout->u.chunk.u.btree.shared = NULL; FUNC_LEAVE_NOAPI(SUCCEED) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 5e45886..a88df43 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -281,6 +281,10 @@ H5FL_DEFINE_STATIC(H5D_chunk_prune_stack_t); * Programmer: Quincey Koziol * Thursday, May 22, 2008 * + * Modifications: + * Vailin Choi; April 2009 + * Reset address and pointer of the array struct for the chunked storage index + * *------------------------------------------------------------------------- */ static herr_t @@ -344,8 +348,8 @@ H5D_chunk_new(H5F_t *f, hid_t dapl_id, hid_t dxpl_id, H5D_t *dset, /* Retain computed chunk size */ H5_ASSIGN_OVERFLOW(dset->shared->layout.u.chunk.size, chunk_size, uint64_t, uint32_t); - /* Reset index address */ - if(H5D_chunk_idx_reset(&dset->shared->layout) < 0) + /* Reset address and pointer of the array struct for the chunked storage index */ + if(H5D_chunk_idx_reset(&dset->shared->layout, TRUE) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, FAIL, "unable to reset chunked storage index") /* Initialize the chunk cache for the dataset */ @@ -1919,11 +1923,15 @@ done: * * Programmer: Quincey Koziol * Thursday, January 15, 2009 + * + * Modifications: + * Vailin Choi; April 2009 + * Pass along RESET_ADDR: whether to reset the address of chunked storage index * *------------------------------------------------------------------------- */ herr_t -H5D_chunk_idx_reset(H5O_layout_t *layout) +H5D_chunk_idx_reset(H5O_layout_t *layout, hbool_t reset_addr) { herr_t ret_value = SUCCEED; /* Return value */ @@ -1938,7 +1946,7 @@ H5D_chunk_idx_reset(H5O_layout_t *layout) H5D_COPS_BTREE == layout->u.chunk.ops)); /* Reset index structures */ - if((layout->u.chunk.ops->reset)(layout) < 0) + if((layout->u.chunk.ops->reset)(layout, reset_addr) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to reset chunk index info") done: diff --git a/src/H5Dearray.c b/src/H5Dearray.c index 35cdad6..7ae62e4 100644 --- a/src/H5Dearray.c +++ b/src/H5Dearray.c @@ -131,7 +131,7 @@ static herr_t H5D_earray_idx_copy_shutdown(H5O_layout_t *layout_src, H5O_layout_t *layout_dst, hid_t dxpl_id); static herr_t H5D_earray_idx_size(const H5D_chk_idx_info_t *idx_info, hsize_t *size); -static herr_t H5D_earray_idx_reset(H5O_layout_t *layout); +static herr_t H5D_earray_idx_reset(H5O_layout_t *layout, hbool_t reset_addr); static herr_t H5D_earray_idx_dump(const H5D_chk_idx_info_t *idx_info, FILE *stream); static herr_t H5D_earray_idx_dest(const H5D_chk_idx_info_t *idx_info); @@ -1409,17 +1409,22 @@ done: * Programmer: Quincey Koziol * Saturday, January 31, 2009 * + * Modifications: + * Vailin Choi; April 2009 + * Reset address of the chunked storage index if RESET_ADDR is set + * *------------------------------------------------------------------------- */ static herr_t -H5D_earray_idx_reset(H5O_layout_t *layout) +H5D_earray_idx_reset(H5O_layout_t *layout, hbool_t reset_addr) { FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5D_earray_idx_reset) HDassert(layout); /* Reset index info */ - layout->u.chunk.u.earray.addr = HADDR_UNDEF; + if(reset_addr) + layout->u.chunk.u.earray.addr = HADDR_UNDEF; layout->u.chunk.u.earray.ea = NULL; FUNC_LEAVE_NOAPI(SUCCEED) diff --git a/src/H5Dpkg.h b/src/H5Dpkg.h index 4122326..b936dba 100644 --- a/src/H5Dpkg.h +++ b/src/H5Dpkg.h @@ -281,7 +281,7 @@ typedef herr_t (*H5D_chunk_copy_shutdown_func_t)(H5O_layout_t *layout_src, H5O_layout_t *layout_dst, hid_t dxpl_id); typedef herr_t (*H5D_chunk_size_func_t)(const H5D_chk_idx_info_t *idx_info, hsize_t *idx_size); -typedef herr_t (*H5D_chunk_reset_func_t)(H5O_layout_t *layout); +typedef herr_t (*H5D_chunk_reset_func_t)(H5O_layout_t *layout, hbool_t reset_addr); typedef herr_t (*H5D_chunk_dump_func_t)(const H5D_chk_idx_info_t *idx_info, FILE *stream); typedef herr_t (*H5D_chunk_dest_func_t)(const H5D_chk_idx_info_t *idx_info); diff --git a/src/H5Dprivate.h b/src/H5Dprivate.h index 7327d0d..ad9c795 100644 --- a/src/H5Dprivate.h +++ b/src/H5Dprivate.h @@ -166,7 +166,7 @@ H5_DLL herr_t H5D_contig_delete(H5F_t *f, hid_t dxpl_id, const H5O_layout_t *layout); /* Functions that operate on chunked storage */ -H5_DLL herr_t H5D_chunk_idx_reset(H5O_layout_t *layout); +H5_DLL herr_t H5D_chunk_idx_reset(H5O_layout_t *layout, hbool_t reset_addr); H5_DLL herr_t H5D_chunk_delete(H5F_t *f, hid_t dxpl_id, H5O_t *oh, H5O_layout_t *layout); diff --git a/src/H5Olayout.c b/src/H5Olayout.c index b92a8bb..c79f328 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -447,6 +447,10 @@ done: * Programmer: Robb Matzke * Wednesday, October 8, 1997 * + * Modifications: + * Vailin Choi; April 2009 + * Reset the pointer of the chunked storage index but not the address + * *------------------------------------------------------------------------- */ static void * @@ -474,8 +478,13 @@ H5O_layout_copy(const void *_mesg, void *_dest) /* Copy over the raw data */ HDmemcpy(dest->u.compact.buf, mesg->u.compact.buf, dest->u.compact.size); + } /* end if */ + /* Reset the pointer of the chunked storage index but not the address */ + if(dest->type == H5D_CHUNKED && dest->u.chunk.ops) + H5D_chunk_idx_reset(dest, FALSE); + /* Set return value */ ret_value = dest; @@ -654,6 +663,10 @@ done: * Programmer: Peter Cao * July 23, 2005 * + * Modifications: + * Vailin Choi; April 2009 + * Reset address and pointer of the array struct for the chunked storage index + * *------------------------------------------------------------------------- */ static void * @@ -713,7 +726,7 @@ H5O_layout_copy_file(H5F_t *file_src, void *mesg_src, H5F_t *file_dst, case H5D_CHUNKED: if(H5D_chunk_is_space_alloc(layout_src)) { /* Layout is not created in the destination file, reset index address */ - if(H5D_chunk_idx_reset(layout_dst) < 0) + if(H5D_chunk_idx_reset(layout_dst, TRUE) < 0) HGOTO_ERROR(H5E_IO, H5E_CANTINIT, NULL, "unable to reset chunked storage index in dest") /* Create chunked layout */ diff --git a/src/H5Pdcpl.c b/src/H5Pdcpl.c index dcd120e..b2bd71d 100644 --- a/src/H5Pdcpl.c +++ b/src/H5Pdcpl.c @@ -223,6 +223,10 @@ done: * Programmer: Raymond Lu * Tuesday, October 2, 2001 * + * Modifications: + * Vailin Choi; April 2009 + * Reset address and pointer of the array struct for the chunked storage index + * *------------------------------------------------------------------------- */ /* ARGSUSED */ @@ -280,7 +284,8 @@ H5P_dcrt_copy(hid_t dst_plist_id, hid_t src_plist_id, void UNUSED *copy_data) /* Reset index info, if the chunk ops are set */ if(dst_layout.u.chunk.ops) - if(H5D_chunk_idx_reset(&dst_layout) < 0) + /* Reset address and pointer of the array struct for the chunked storage index */ + if(H5D_chunk_idx_reset(&dst_layout, TRUE) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTINIT, FAIL, "unable to reset chunked storage index in dest") /* Reset chunk index ops */ diff --git a/test/dsets.c b/test/dsets.c index 912098f..2bb124f 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -7094,6 +7094,127 @@ error: return -1; } /* end test_chunk_fast() */ +/*------------------------------------------------------------------------- + * Function: test_reopen_chunk_fast + * + * Purpose: To verify a bug in extensible arrays as chunk index. + * When the dataset is closed in H5D_close(), the pointer + * to the extensible array struct in the layout message + * is copied via H5D_flush_real() before H5D_chunk_dest(). + * This causes an abort from "Assertion `ea->hdr' failed." + * later when the dataset is re-opened and read. + * + * Return: Success: 0 + * Failure: -1 + * + * Programmer: Vailin Choi + * April 13, 2009 + * + *------------------------------------------------------------------------- + */ +static herr_t +test_reopen_chunk_fast(hid_t fapl) +{ + char filename[FILENAME_BUF_SIZE]; + hid_t fid = -1; /* File ID */ + hid_t dcpl = -1; /* Dataset creation property list ID */ + hid_t sid = -1; /* Dataspace ID */ + hid_t scalar_sid = -1;/* Scalar dataspace ID */ + hid_t dsid = -1; /* Dataset ID */ + hsize_t dim, max_dim, chunk_dim; /* Dataset and chunk dimensions */ + hsize_t hs_offset; /* Hyperslab offset */ + hsize_t hs_size; /* Hyperslab size */ + H5D_alloc_time_t alloc_time; /* Storage allocation time */ + unsigned write_elem, read_elem; /* Element written/read */ + unsigned u; /* Local index variable */ + + TESTING("datasets w/extensible array open/reopen with read/write"); + + h5_fixname(FILENAME[9], fapl, filename, sizeof filename); + + /* Loop over storage allocation time */ + for(alloc_time = H5D_ALLOC_TIME_EARLY; alloc_time <= H5D_ALLOC_TIME_INCR; alloc_time++) { + /* Create file */ + if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) FAIL_STACK_ERROR + + /* Create dataset creation property list */ + if((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) FAIL_STACK_ERROR + + /* Set chunking */ + chunk_dim = 10; + if(H5Pset_chunk(dcpl, 1, &chunk_dim) < 0) FAIL_STACK_ERROR + + /* Set fill time */ + if(H5Pset_fill_time(dcpl, H5D_FILL_TIME_ALLOC) < 0) FAIL_STACK_ERROR + + /* Set allocation time */ + if(H5Pset_alloc_time(dcpl, alloc_time) < 0) FAIL_STACK_ERROR + + /* Create scalar dataspace */ + if((scalar_sid = H5Screate(H5S_SCALAR)) < 0) FAIL_STACK_ERROR + + /* Create 1-D dataspace */ + dim = 100; + max_dim = H5S_UNLIMITED; + if((sid = H5Screate_simple(1, &dim, &max_dim)) < 0) FAIL_STACK_ERROR + + /* Create chunked dataset */ + if((dsid = H5Dcreate2(fid, "dset", H5T_NATIVE_UINT, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) + FAIL_STACK_ERROR + + /* Fill existing elements */ + hs_size = 1; + for(u = 0; u < 100; u++) { + /* Select a single element in the dataset */ + hs_offset = u; + if(H5Sselect_hyperslab(sid, H5S_SELECT_SET, &hs_offset, NULL, &hs_size, NULL) < 0) + FAIL_STACK_ERROR + /* Write element to dataset */ + write_elem = u; + if(H5Dwrite(dsid, H5T_NATIVE_UINT, scalar_sid, sid, H5P_DEFAULT, &write_elem) < 0) + FAIL_STACK_ERROR + } /* end for */ + + /* Close everything */ + if(H5Dclose(dsid) < 0) FAIL_STACK_ERROR + + /* Reopen the dataset */ + if((dsid = H5Dopen2(fid, "dset", H5P_DEFAULT)) < 0) FAIL_STACK_ERROR + hs_size = 1; + + /* Read from dataset */ + for(u = 0; u < 100; u++) { + /* Select a single element in the dataset */ + hs_offset = u; + if(H5Sselect_hyperslab(sid, H5S_SELECT_SET, &hs_offset, NULL, &hs_size, NULL) < 0) + FAIL_STACK_ERROR + + /* Read element from dataset */ + if(H5Dread(dsid, H5T_NATIVE_UINT, scalar_sid, sid, H5P_DEFAULT, &read_elem) < 0) + FAIL_STACK_ERROR + } /* end for */ + + if(H5Dclose(dsid) < 0) FAIL_STACK_ERROR + if(H5Sclose(sid) < 0) FAIL_STACK_ERROR + if(H5Sclose(scalar_sid) < 0) FAIL_STACK_ERROR + if(H5Pclose(dcpl) < 0) FAIL_STACK_ERROR + if(H5Fclose(fid) < 0) FAIL_STACK_ERROR + + } /* end for */ + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY { + H5Pclose(dcpl); + H5Dclose(dsid); + H5Sclose(sid); + H5Sclose(scalar_sid); + H5Fclose(fid); + } H5E_END_TRY; + return -1; +} /* end test_reopen_chunk_fast() */ /*------------------------------------------------------------------------- * Function: main @@ -7218,6 +7339,8 @@ main(void) nerrors += (test_chunk_cache(my_fapl) < 0 ? 1 : 0); nerrors += (test_big_chunks_bypass_cache(my_fapl) < 0 ? 1 : 0); nerrors += (test_chunk_fast(my_fapl) < 0 ? 1 : 0); + nerrors += (test_reopen_chunk_fast(my_fapl) < 0 ? 1 : 0); + if(H5Fclose(file) < 0) goto error; -- cgit v0.12