[svn-r17624] Description:

	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

7 files changed, 187 insertions, 67 deletions

diff --git a/MANIFEST b/MANIFEST
index 16714aa..1c69313 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -851,6 +851,7 @@
 ./test/gen_old_group.c			_DO_NOT_DISTRIBUTE_
 ./test/gen_old_layout.c			_DO_NOT_DISTRIBUTE_
 ./test/gen_old_mtime.c			_DO_NOT_DISTRIBUTE_
+./test/gen_specmetaread.c		_DO_NOT_DISTRIBUTE_
 ./test/gen_udlinks.c			_DO_NOT_DISTRIBUTE_
 ./test/getname.c
 ./test/gheap.c
@@ -876,6 +877,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/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/Makefile.am b/test/Makefile.am
index d233f96..ec1eed7 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -62,7 +62,7 @@ check_PROGRAMS=$(TEST_PROG) error_test err_compat tcheck_version testmeta
 # so do not appear in this list.
 BUILD_ALL_PROGS=gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_new_array \
     gen_new_fill gen_new_group gen_new_mtime gen_new_super gen_noencoder \
-    gen_nullspace gen_udlinks space_overflow gen_filespace
+    gen_nullspace gen_udlinks space_overflow gen_filespace gen_specmetaread
 
 if BUILD_ALL_CONDITIONAL
   noinst_PROGRAMS=$(BUILD_ALL_PROGS)
diff --git a/test/Makefile.in b/test/Makefile.in
index e1ed182..3f063e9 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -92,7 +92,8 @@ am__EXEEXT_2 = gen_bad_ohdr$(EXEEXT) gen_bogus$(EXEEXT) \
 	gen_new_group$(EXEEXT) gen_new_mtime$(EXEEXT) \
 	gen_new_super$(EXEEXT) gen_noencoder$(EXEEXT) \
 	gen_nullspace$(EXEEXT) gen_udlinks$(EXEEXT) \
-	space_overflow$(EXEEXT) gen_filespace$(EXEEXT)
+	space_overflow$(EXEEXT) gen_filespace$(EXEEXT) \
+	gen_specmetaread$(EXEEXT)
 PROGRAMS = $(noinst_PROGRAMS)
 app_ref_SOURCES = app_ref.c
 app_ref_OBJECTS = app_ref.$(OBJEXT)
@@ -246,6 +247,10 @@ gen_nullspace_SOURCES = gen_nullspace.c
 gen_nullspace_OBJECTS = gen_nullspace.$(OBJEXT)
 gen_nullspace_LDADD = $(LDADD)
 gen_nullspace_DEPENDENCIES = libh5test.la $(LIBHDF5)
+gen_specmetaread = gen_specmetaread.c
+gen_specmetaread_OBJECTS = gen_specmetaread.$(OBJEXT)
+gen_specmetaread_LDADD = $(LDADD)
+gen_specmetaread_DEPENDENCIES = libh5test.la $(LIBHDF5)
 gen_udlinks_SOURCES = gen_udlinks.c
 gen_udlinks_OBJECTS = gen_udlinks.$(OBJEXT)
 gen_udlinks_LDADD = $(LDADD)
@@ -377,7 +382,7 @@ SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c btree2.c \
 	mount.c mtime.c ntypes.c objcopy.c ohdr.c pool.c reserved.c \
 	set_extent.c space_overflow.c stab.c tcheck_version.c \
 	$(testhdf5_SOURCES) testmeta.c $(ttsafe_SOURCES) unlink.c \
-	vfd.c
+	vfd.c gen_specmetaread.c
 DIST_SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c \
 	btree2.c cache.c cache_api.c cmpd_dset.c cross_read.c dangle.c \
 	dsets.c dt_arith.c dtransform.c dtypes.c earray.c enum.c \
@@ -390,7 +395,7 @@ DIST_SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c \
 	istore.c lheap.c links.c mf.c mount.c mtime.c ntypes.c \
 	objcopy.c ohdr.c pool.c reserved.c set_extent.c \
 	space_overflow.c stab.c tcheck_version.c $(testhdf5_SOURCES) \
-	testmeta.c $(ttsafe_SOURCES) unlink.c vfd.c
+	testmeta.c $(ttsafe_SOURCES) unlink.c vfd.c gen_specmetaread.c
 ETAGS = etags
 CTAGS = ctags
 am__tty_colors = \
@@ -712,7 +717,7 @@ TEST_PROG = testhdf5 lheap ohdr stab gheap cache cache_api \
 # so do not appear in this list.
 BUILD_ALL_PROGS = gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_new_array \
     gen_new_fill gen_new_group gen_new_mtime gen_new_super gen_noencoder \
-    gen_nullspace gen_udlinks space_overflow gen_filespace
+    gen_nullspace gen_udlinks space_overflow gen_filespace gen_specmetaread
 
 
 # The libh5test library provides common support code for the tests.
@@ -937,6 +942,9 @@ gen_noencoder$(EXEEXT): $(gen_noencoder_OBJECTS) $(gen_noencoder_DEPENDENCIES)
 gen_nullspace$(EXEEXT): $(gen_nullspace_OBJECTS) $(gen_nullspace_DEPENDENCIES) 
 	@rm -f gen_nullspace$(EXEEXT)
 	$(LINK) $(gen_nullspace_OBJECTS) $(gen_nullspace_LDADD) $(LIBS)
+gen_specmetaread$(EXEEXT): $(gen_specmetaread_OBJECTS) $(gen_specmetaread_DEPENDENCIES) 
+	@rm -f gen_specmetaread$(EXEEXT)
+	$(LINK) $(gen_specmetaread_OBJECTS) $(gen_specmetaread_LDADD) $(LIBS)
 gen_udlinks$(EXEEXT): $(gen_udlinks_OBJECTS) $(gen_udlinks_DEPENDENCIES) 
 	@rm -f gen_udlinks$(EXEEXT)
 	$(LINK) $(gen_udlinks_OBJECTS) $(gen_udlinks_LDADD) $(LIBS)
@@ -1055,6 +1063,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_new_super.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_noencoder.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_nullspace.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_specmetaread.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_udlinks.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/getname.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gheap.Po@am__quote@
diff --git a/test/gen_specmetaread.c b/test/gen_specmetaread.c
new file mode 100644
index 0000000..f489119
--- /dev/null
+++ b/test/gen_specmetaread.c
@@ -0,0 +1,96 @@
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Copyright by The HDF Group.                                               *
+ * Copyright by the Board of Trustees of the University of Illinois.         *
+ * All rights reserved.                                                      *
+ *                                                                           *
+ * This file is part of HDF5.  The full HDF5 copyright notice, including     *
+ * terms governing use, modification, and redistribution, is contained in    *
+ * the files COPYING and Copyright.html.  COPYING can be found at the root   *
+ * of the source code distribution tree; Copyright.html can be found at the  *
+ * root level of an installed copy of the electronic HDF5 document set and   *
+ * is linked from the top-level documents page.  It can also be found at     *
+ * http://hdfgroup.org/HDF5/doc/Copyright.html.  If you do not have          *
+ * access to either file, you may request a copy from help@hdfgroup.org.     *
+ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+/*
+ * Programmer:  Quincey Koziol <koziol@hdfgroup.org>
+ *              Thursday, October  8, 2009
+ *
+ * Purpose:	Create a file with a dataset who's raw data immediately follows
+ *      its object header, so that when the dataset is unlinked from its parent
+ *      group, a speculative read of the object header would get the raw data
+ *      into the metadata accumulator, "polluting" it.
+ *		To build the test file, this program MUST be compiled and linked with
+ *      the library on the trunk as of when this file is checked in.
+ */
+
+#include "hdf5.h"
+#include <assert.h>
+
+#define FILENAME        "specmetaread.h5"
+#define DIM             10
+
+int
+main(void)
+{
+    hid_t	fid;
+    hid_t	fapl;
+    hid_t       did;
+    hid_t 	space;
+    hsize_t     dim[1] = {DIM};
+    unsigned    data[DIM];
+    unsigned    u;
+    herr_t      ret;         /* Generic return value */
+
+    /* Initialize the data */
+    for(u = 0; u < DIM; u++)
+	data[u] = u;
+
+    /* Create a FAPL with the metadata and small data aggregators turned off */
+    fapl = H5Pcreate(H5P_FILE_ACCESS);
+    assert(fapl > 0);
+    ret = H5Pset_meta_block_size(fapl, (hsize_t)0);
+    assert(ret >= 0);
+    ret = H5Pset_small_data_block_size(fapl, (hsize_t)0);
+    assert(ret >= 0);
+
+    /* Create file */
+    fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, fapl);
+    assert(fid > 0);
+
+    /* Close FAPL */
+    ret = H5Pclose(fapl);
+    assert(ret >= 0);
+
+    /* Create dataspace */
+    space = H5Screate_simple(1, dim, NULL);
+    assert(space > 0);
+
+    /* Create dataset #1 */
+    did = H5Dcreate2(fid, "dset1", H5T_NATIVE_UINT, space, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    assert(did > 0);
+    ret = H5Dwrite(did, H5T_NATIVE_UINT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);
+    assert(ret >= 0);
+    ret = H5Dclose(did);
+    assert(ret >= 0);
+
+    /* Create dataset #2 */
+    did = H5Dcreate2(fid, "dset2", H5T_NATIVE_UINT, space, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    assert(did > 0);
+    ret = H5Dwrite(did, H5T_NATIVE_UINT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);
+    assert(ret >= 0);
+    ret = H5Dclose(did);
+    assert(ret >= 0);
+
+    /* Close dataspace */
+    ret = H5Sclose(space);
+    assert(ret >= 0);
+
+    /* Close file */
+    ret = H5Fclose(fid);
+    assert(ret >= 0);
+
+    return 0;
+}
+
diff --git a/test/specmetaread.h5 b/test/specmetaread.h5
new file mode 100644
index 0000000..50b0e69
--- /dev/null
+++ b/test/specmetaread.h5
diff --git a/test/tmisc.c b/test/tmisc.c
index 41128a2..21715b4 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
@@ -5106,6 +5114,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.
@@ -5149,6 +5214,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() */
 
@@ -5203,5 +5269,6 @@ cleanup_misc(void)
     HDremove(MISC25C_FILE);
     HDremove(MISC26_FILE);
     HDremove(MISC28_FILE);
+    HDremove(MISC29_COPY_FILE);
 }