From 2a2a49f42280220ad7cf2c45b1e5103d3b72146a Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Tue, 13 Oct 2009 10:28:42 -0500 Subject: [svn-r17638] Description: Bring r17624 from trunk to 1.8 branch: Don't allow reads to change or add to the metadata accumulator, since they might be speculative and could bring raw data into the metadata accumulator. Tested on: FreeBSD/32 6.3 (duty) in debug mode FreeBSD/64 6.3 (liberty) w/C++ & FORTRAN, in debug mode Linux/32 2.6 (jam) w/PGI compilers, w/default API=1.8.x, w/C++ & FORTRAN, w/threadsafe, in debug mode Linux/64-amd64 2.6 (smirom) w/Intel compilers, w/default API=1.6.x, w/C++ & FORTRAN, in production mode Solaris/32 2.10 (linew) w/deprecated symbols disabled, w/C++ & FORTRAN, w/szip filter, in production mode Linux/64-ia64 2.6 (cobalt) w/Intel compilers, w/C++ & FORTRAN, in production mode Linux/64-ia64 2.4 (tg-login3) w/parallel, w/FORTRAN, in debug mode Linux/64-amd64 2.6 (abe) w/parallel, w/FORTRAN, in production mode Mac OS X/32 10.6.1 (amazon) in debug mode Mac OS X/32 10.6.1 (amazon) w/C++ & FORTRAN, w/threadsafe, in production mode --- MANIFEST | 1 + configure | 2 +- release_docs/RELEASE.txt | 3 ++ src/H5Faccum.c | 70 ++++++----------------------------------------- test/specmetaread.h5 | Bin 0 -> 1752 bytes test/tmisc.c | 67 +++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+), 63 deletions(-) create mode 100644 test/specmetaread.h5 diff --git a/MANIFEST b/MANIFEST index deead23..a8f573a 100644 --- a/MANIFEST +++ b/MANIFEST @@ -848,6 +848,7 @@ ./test/pool.c ./test/set_extent.c ./test/space_overflow.c _DO_NOT_DISTRIBUTE_ +./test/specmetaread.h5 ./test/stab.c ./test/tarray.c ./test/tarrold.h5 diff --git a/configure b/configure index 664c7bc..e410924 100755 --- a/configure +++ b/configure @@ -1,5 +1,5 @@ #! /bin/sh -# From configure.in Id: configure.in 17548 2009-09-29 16:45:07Z acheng . +# From configure.in Id: configure.in 17568 2009-10-01 17:06:48Z mamcgree . # Guess values for system-dependent variables and create Makefiles. # Generated by GNU Autoconf 2.64 for HDF5 1.8.3-snap13. # diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 95f9795..bf71c3c 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -128,6 +128,9 @@ Bug Fixes since HDF5-1.8.3 Library ------- + - Fixed error in library's internal caching mechanisms which could cause + an assertion failure (and attendent core dump) when encountering an + unusually formatted file. (QAK - 2009/10/13) - Fixed incorrect return value for H5Pget_preserve. AKC - 2009/10/08 - 1628 - Fixed an assertion failure that occurred when H5Ocopy was called on a dataset using a vlen inside a compound. NAF - 2009/10/02 - 1597 diff --git a/src/H5Faccum.c b/src/H5Faccum.c index d8b9820..e525a08 100644 --- a/src/H5Faccum.c +++ b/src/H5Faccum.c @@ -99,6 +99,10 @@ H5FL_BLK_DEFINE_STATIC(meta_accum); * Purpose: Attempts to read some data from the metadata accumulator for * a file into a buffer. * + * Note: We can't change (or add to) the metadata accumulator, because + * this might be a speculative read and could possibly read raw + * data into the metadata accumulator. + * * Return: Non-negative on success/Negative on failure * * Programmer: Quincey Koziol @@ -185,69 +189,11 @@ H5F_accum_read(const H5F_t *f, hid_t dxpl_id, H5FD_mem_t type, haddr_t addr, /* Make certain we've read it all */ HDassert(size == 0); } /* end if */ - /* Current read doesn't overlap with metadata accumulator, read it into accumulator */ + /* Current read doesn't overlap with metadata accumulator, read it from file */ else { - /* Only update the metadata accumulator if it is not dirty or if - * we are allowed to write the accumulator out during reads (when - * it is dirty) - */ - if(f->shared->feature_flags & H5FD_FEAT_ACCUMULATE_METADATA_READ || !f->shared->accum.dirty) { - /* Flush current contents, if dirty */ - if(f->shared->accum.dirty) { - if(H5FD_write(f->shared->lf, dxpl_id, H5FD_MEM_DEFAULT, f->shared->accum.loc, f->shared->accum.size, f->shared->accum.buf) < 0) - HGOTO_ERROR(H5E_IO, H5E_WRITEERROR, FAIL, "driver write request failed") - - /* Reset accumulator dirty flag */ - f->shared->accum.dirty = FALSE; - } /* end if */ - - /* Cache the new piece of metadata */ - /* Check if we need to resize the buffer */ - if(size > f->shared->accum.alloc_size) { - size_t new_size; /* New size of accumulator */ - - /* Adjust the buffer size to be a power of 2 that is large enough to hold data */ - new_size = (size_t)1 << (1 + H5V_log2_gen((uint64_t)(size - 1))); - - /* Grow the metadata accumulator buffer */ - if(NULL == (f->shared->accum.buf = H5FL_BLK_REALLOC(meta_accum, f->shared->accum.buf, new_size))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate metadata accumulator buffer") - - /* Note the new buffer size */ - f->shared->accum.alloc_size = new_size; - } /* end if */ - else { - /* Check if we should shrink the accumulator buffer */ - if(size < (f->shared->accum.alloc_size / H5F_ACCUM_THROTTLE) && - f->shared->accum.alloc_size > H5F_ACCUM_THRESHOLD) { - size_t new_size = (f->shared->accum.alloc_size / H5F_ACCUM_THROTTLE); /* New size of accumulator buffer */ - - /* Shrink the accumulator buffer */ - if(NULL == (f->shared->accum.buf = H5FL_BLK_REALLOC(meta_accum, f->shared->accum.buf, new_size))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate metadata accumulator buffer") - - /* Note the new buffer size */ - f->shared->accum.alloc_size = new_size; - } /* end if */ - } /* end else */ - - /* Update accumulator information */ - f->shared->accum.loc = addr; - f->shared->accum.size = size; - f->shared->accum.dirty = FALSE; - - /* Read into accumulator */ - if(H5FD_read(f->shared->lf, dxpl_id, H5FD_MEM_DEFAULT, f->shared->accum.loc, f->shared->accum.size, f->shared->accum.buf) < 0) - HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "driver read request failed") - - /* Copy into buffer */ - HDmemcpy(buf, f->shared->accum.buf, size); - } /* end if */ - else { - /* Dispatch to driver */ - if(H5FD_read(f->shared->lf, dxpl_id, type, addr, size, buf) < 0) - HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "driver read request failed") - } /* end else */ + /* Dispatch to driver */ + if(H5FD_read(f->shared->lf, dxpl_id, type, addr, size, buf) < 0) + HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "driver read request failed") } /* end else */ /* Indicate success */ diff --git a/test/specmetaread.h5 b/test/specmetaread.h5 new file mode 100644 index 0000000..50b0e69 Binary files /dev/null and b/test/specmetaread.h5 differ diff --git a/test/tmisc.c b/test/tmisc.c index b646a69..005e77a 100644 --- a/test/tmisc.c +++ b/test/tmisc.c @@ -32,6 +32,9 @@ #include "testhdf5.h" #include "H5Dpkg.h" /* Datasets */ +#define NAME_BUF_SIZE 1024 +#define READ_OLD_BUFSIZE 1024 + /* Definitions for misc. test #1 */ #define MISC1_FILE "tmisc1.h5" #define MISC1_VAL (13417386) /* 0xccbbaa */ @@ -305,6 +308,11 @@ unsigned m13_rdata[MISC13_DIM1][MISC13_DIM2]; /* Data read from dataset #define MISC28_SIZE 10 #define MISC28_NSLOTS 10000 +/* Definitions for misc. test #29 */ +#define MISC29_ORIG_FILE "specmetaread.h5" +#define MISC29_COPY_FILE "tmisc29.h5" +#define MISC29_DSETNAME "dset2" + /**************************************************************** ** ** test_misc1(): test unlinking a dataset from a group and immediately @@ -5118,6 +5126,63 @@ test_misc28(void) CHECK_I(ret, "H5Pclose"); } /* end test_misc28() */ + +/**************************************************************** +** +** test_misc29(): Ensure that speculative metadata reads don't +** get raw data into the metadata accumulator. +** +****************************************************************/ +static void +test_misc29(void) +{ + int fd_old = (-1), fd_new = (-1); /* File descriptors for copying data */ + char buf[READ_OLD_BUFSIZE]; /* Buffer for copying data */ + ssize_t nread; /* Number of bytes read in */ + char *srcdir = HDgetenv("srcdir"); /* where the src code is located */ + char filename[NAME_BUF_SIZE] = ""; /* old test file name */ + hid_t fid; /* File ID */ + herr_t ret; /* Generic return value */ + + /* Output message about test being performed */ + MESSAGE(5, ("Speculative metadata reads\n")); + + /* Generate correct name for test file by prepending the source path */ + if(srcdir && ((HDstrlen(srcdir) + HDstrlen(MISC29_ORIG_FILE) + 1) < sizeof(filename))) { + HDstrcpy(filename, srcdir); + HDstrcat(filename, "/"); + } /* end if */ + HDstrcat(filename, MISC29_ORIG_FILE); + + /* Copy old file into temporary file */ + fd_old = HDopen(filename, O_RDONLY, 0666); + CHECK(fd_old, -1, "HDopen"); + fd_new = HDopen(MISC29_COPY_FILE, O_RDWR|O_CREAT|O_TRUNC, 0666); + CHECK(fd_new, -1, "HDopen"); + + /* Copy data */ + while((nread = HDread(fd_old, buf, (size_t)READ_OLD_BUFSIZE)) > 0) + HDwrite(fd_new, buf, (size_t)nread); + + /* Close files */ + ret = HDclose(fd_old); + CHECK(ret, -1, "HDclose"); + ret = HDclose(fd_new); + CHECK(ret, -1, "HDclose"); + + /* Open the copied file */ + fid = H5Fopen(MISC29_COPY_FILE, H5F_ACC_RDWR, H5P_DEFAULT); + CHECK(fid, FAIL, "H5Fopen"); + + /* Delete the last dataset */ + ret = H5Ldelete(fid, MISC29_DSETNAME, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Ldelete"); + + /* Close the file */ + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); +} /* end test_misc29() */ + /**************************************************************** ** ** test_misc(): Main misc. test routine. @@ -5161,6 +5226,7 @@ test_misc(void) test_misc26(); /* Test closing property lists with long filter pipelines */ test_misc27(); /* Test opening file with object that has bad # of object header messages */ test_misc28(); /* Test that chunks are cached appropriately */ + test_misc29(); /* Test that speculative metadata reads are handled correctly */ } /* test_misc() */ @@ -5216,5 +5282,6 @@ cleanup_misc(void) HDremove(MISC25C_FILE); HDremove(MISC26_FILE); HDremove(MISC28_FILE); + HDremove(MISC29_COPY_FILE); } -- cgit v0.12