From 979873f81d9825fa726ebc126675d60fad7962b2 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Wed, 17 Oct 2018 11:43:14 -0600 Subject: [PATCH 01/30] Fix provenance memory leak --- libhdf5/hdf5file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 47001d0e8..61f8594a8 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -207,7 +207,7 @@ nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio* memio) /* Free the fileinfo struct, which holds info from the fileinfo * hidden attribute. */ if (h5->provenance) - free(h5->provenance); + NC4_free_provenance(h5->provenance); /* Close hdf file. It may not be open, since this function is also * called by NC_create() when a file opening is aborted. */ From c90ab24b48cabf2860fe0b5ee0b6c07763bc12ba Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 18 Oct 2018 03:17:38 -0600 Subject: [PATCH 02/30] moving towards separating HDF5 file close from netcdf4 file close --- include/nc4internal.h | 3 ++- libhdf5/hdf5create.c | 2 +- libhdf5/hdf5file.c | 53 ++++++++++++++++++++++++++++++++++++++++--- libhdf5/hdf5open.c | 2 +- 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/include/nc4internal.h b/include/nc4internal.h index 54e85c149..32c0a3f10 100644 --- a/include/nc4internal.h +++ b/include/nc4internal.h @@ -410,7 +410,8 @@ int nc4_check_dup_name(NC_GRP_INFO_T *grp, char *norm_name); int nc4_get_default_fill_value(const NC_TYPE_INFO_T *type_info, void *fill_value); /* Close the file. */ -int nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio*); +int nc4_close_hdf5_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio); +int nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio); /* HDF5 initialization */ extern int nc4_hdf5_initialized; diff --git a/libhdf5/hdf5create.c b/libhdf5/hdf5create.c index 7c1af0f94..dbbb5fe81 100644 --- a/libhdf5/hdf5create.c +++ b/libhdf5/hdf5create.c @@ -240,7 +240,7 @@ exit: /*failure exit*/ #endif if (fapl_id != H5P_DEFAULT) H5Pclose(fapl_id); if(!nc4_info) return retval; - nc4_close_netcdf4_file(nc4_info,1,NULL); /* treat like abort */ + nc4_close_hdf5_file(nc4_info, 1, NULL); /* treat like abort */ return retval; } diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 47001d0e8..8d7ba954c 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -161,7 +161,7 @@ sync_netcdf4_file(NC_FILE_INFO_T *h5) * @author Ed Hartnett, Dennis Heimbigner */ int -nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio* memio) +nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio) { NC_HDF5_FILE_INFO_T *hdf5_info; int retval; @@ -247,6 +247,53 @@ nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio* memio) return NC_NOERR; } +/** + * @internal This function will recurse through an open HDF5 file and + * release resources. All open HDF5 objects in the file will be + * closed. + * + * @param h5 Pointer to HDF5 file info struct. + * @param abort True if this is an abort. + * @param memio the place to return a core image if not NULL + * + * @return ::NC_NOERR No error. + * @return ::NC_EHDFERR HDF5 could not close the file. + * @author Ed Hartnett + */ +int +nc4_close_hdf5_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio) +{ + NC_HDF5_FILE_INFO_T *hdf5_info; + int retval; + + assert(h5 && h5->root_grp && h5->format_file_info); + LOG((3, "%s: h5->path %s abort %d", __func__, h5->controller->path, abort)); + + /* /\* Get HDF5 specific info. *\/ */ + /* hdf5_info = (NC_HDF5_FILE_INFO_T *)h5->format_file_info; */ + + /* /\* According to the docs, always end define mode on close. *\/ */ + /* if (h5->flags & NC_INDEF) */ + /* h5->flags ^= NC_INDEF; */ + + /* /\* Sync the file, unless we're aborting, or this is a read-only */ + /* * file. *\/ */ + /* if (!h5->no_write && !abort) */ + /* if ((retval = sync_netcdf4_file(h5))) */ + /* return retval; */ + + /* /\* Close all open HDF5 objects within the file. *\/ */ + /* if ((retval = nc4_rec_grp_HDF5_del(h5->root_grp))) */ + /* return retval; */ + + /* Release all intarnal lists and metadata associated with this + * file. All HDF5 objects have already been released. */ + if ((retval = nc4_close_netcdf4_file(h5, abort, memio))) + return retval; + + return NC_NOERR; +} + /** * @internal Output a list of still-open objects in the HDF5 * file. This is only called if the file fails to close cleanly. @@ -510,7 +557,7 @@ NC4_abort(int ncid) /* Free any resources the netcdf-4 library has for this file's * metadata. */ - if ((retval = nc4_close_netcdf4_file(nc4_info, 1, NULL))) + if ((retval = nc4_close_hdf5_file(nc4_info, 1, NULL))) return retval; /* Delete the file, if we should. */ @@ -559,7 +606,7 @@ NC4_close(int ncid, void* params) } /* Call the nc4 close. */ - if ((retval = nc4_close_netcdf4_file(grp->nc4_info, 0, memio))) + if ((retval = nc4_close_hdf5_file(grp->nc4_info, 0, memio))) return retval; return NC_NOERR; diff --git a/libhdf5/hdf5open.c b/libhdf5/hdf5open.c index 9830b51a7..88d4192f4 100644 --- a/libhdf5/hdf5open.c +++ b/libhdf5/hdf5open.c @@ -552,7 +552,7 @@ exit: if (fapl_id > 0 && fapl_id != H5P_DEFAULT) H5Pclose(fapl_id); if (nc4_info) - nc4_close_netcdf4_file(nc4_info, 1, 0); /* treat like abort*/ + nc4_close_hdf5_file(nc4_info, 1, 0); /* treat like abort*/ return retval; } From fa86d3c488502073aef6375299d70f21f46f911b Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 18 Oct 2018 03:20:39 -0600 Subject: [PATCH 03/30] continuing to separate hdf5/libsrc4 file close code --- libhdf5/hdf5file.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 8d7ba954c..82af6c9c1 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -172,15 +172,15 @@ nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio) /* Get HDF5 specific info. */ hdf5_info = (NC_HDF5_FILE_INFO_T *)h5->format_file_info; - /* According to the docs, always end define mode on close. */ - if (h5->flags & NC_INDEF) - h5->flags ^= NC_INDEF; + /* /\* According to the docs, always end define mode on close. *\/ */ + /* if (h5->flags & NC_INDEF) */ + /* h5->flags ^= NC_INDEF; */ - /* Sync the file, unless we're aborting, or this is a read-only - * file. */ - if (!h5->no_write && !abort) - if ((retval = sync_netcdf4_file(h5))) - return retval; + /* /\* Sync the file, unless we're aborting, or this is a read-only */ + /* * file. *\/ */ + /* if (!h5->no_write && !abort) */ + /* if ((retval = sync_netcdf4_file(h5))) */ + /* return retval; */ /* Delete all the list contents for vars, dims, and atts, in each * group. */ @@ -269,18 +269,18 @@ nc4_close_hdf5_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio) assert(h5 && h5->root_grp && h5->format_file_info); LOG((3, "%s: h5->path %s abort %d", __func__, h5->controller->path, abort)); - /* /\* Get HDF5 specific info. *\/ */ - /* hdf5_info = (NC_HDF5_FILE_INFO_T *)h5->format_file_info; */ + /* Get HDF5 specific info. */ + hdf5_info = (NC_HDF5_FILE_INFO_T *)h5->format_file_info; - /* /\* According to the docs, always end define mode on close. *\/ */ - /* if (h5->flags & NC_INDEF) */ - /* h5->flags ^= NC_INDEF; */ + /* According to the docs, always end define mode on close. */ + if (h5->flags & NC_INDEF) + h5->flags ^= NC_INDEF; - /* /\* Sync the file, unless we're aborting, or this is a read-only */ - /* * file. *\/ */ - /* if (!h5->no_write && !abort) */ - /* if ((retval = sync_netcdf4_file(h5))) */ - /* return retval; */ + /* Sync the file, unless we're aborting, or this is a read-only + * file. */ + if (!h5->no_write && !abort) + if ((retval = sync_netcdf4_file(h5))) + return retval; /* /\* Close all open HDF5 objects within the file. *\/ */ /* if ((retval = nc4_rec_grp_HDF5_del(h5->root_grp))) */ From 695e295734ea40208614b5229036e5591dd4b87a Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 18 Oct 2018 03:29:21 -0600 Subject: [PATCH 04/30] now calling recursive function to close HDF5 objects in file --- libhdf5/hdf5file.c | 6 +- libhdf5/hdf5internal.c | 123 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index 82af6c9c1..f38fb1e12 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -282,9 +282,9 @@ nc4_close_hdf5_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio) if ((retval = sync_netcdf4_file(h5))) return retval; - /* /\* Close all open HDF5 objects within the file. *\/ */ - /* if ((retval = nc4_rec_grp_HDF5_del(h5->root_grp))) */ - /* return retval; */ + /* Close all open HDF5 objects within the file. */ + if ((retval = nc4_rec_grp_HDF5_del(h5->root_grp))) + return retval; /* Release all intarnal lists and metadata associated with this * file. All HDF5 objects have already been released. */ diff --git a/libhdf5/hdf5internal.c b/libhdf5/hdf5internal.c index 412362ed6..4a09cd18f 100644 --- a/libhdf5/hdf5internal.c +++ b/libhdf5/hdf5internal.c @@ -466,6 +466,129 @@ exit: return retval; } +/** + * @internal Recursively free HDF5 objects for a group (and everything + * it contains). + * + * @param grp Pointer to group info struct. + * + * @return ::NC_NOERR No error. + * @author Ed Hartnett + */ +int +nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp) +{ + NC_VAR_INFO_T *var; + NC_DIM_INFO_T *dim; + NC_ATT_INFO_T *att; + int a; + int i; + int retval; + + assert(grp); + LOG((3, "%s: grp->name %s", __func__, grp->hdr.name)); + + /* Recursively call this function for each child, if any, stopping + * if there is an error. */ + for (i = 0; i < ncindexsize(grp->children); i++) + if ((retval = nc4_rec_grp_HDF5_del((NC_GRP_INFO_T *)ncindexith(grp->children, + i)))) + return retval; + + /* /\* Close HDF5 resources associated with attributes. *\/ */ + /* for (a = 0; a < ncindexsize(grp->att); a++) */ + /* { */ + /* att = (NC_ATT_INFO_T *)ncindexith(grp->att, a); */ + /* assert(att); */ + + /* /\* Close the HDF5 typeid. *\/ */ + /* if (att->native_hdf_typeid && H5Tclose(att->native_hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ + /* } */ + + /* /\* Close HDF5 resources associated with vars. *\/ */ + /* for (i = 0; i < ncindexsize(grp->vars); i++) */ + /* { */ + /* var = (NC_VAR_INFO_T *)ncindexith(grp->vars, i); */ + /* assert(var); */ + + /* /\* Close the HDF5 dataset associated with this var. *\/ */ + /* if (var->hdf_datasetid) */ + /* { */ + /* LOG((3, "closing HDF5 dataset %lld", var->hdf_datasetid)); */ + /* if (H5Dclose(var->hdf_datasetid) < 0) */ + /* return NC_EHDFERR; */ + /* } */ + + /* for (a = 0; a < ncindexsize(var->att); a++) */ + /* { */ + /* att = (NC_ATT_INFO_T *)ncindexith(var->att, a); */ + /* assert(att); */ + + /* /\* Close the HDF5 typeid if one is open. *\/ */ + /* if (att->native_hdf_typeid && H5Tclose(att->native_hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ + /* } */ + /* } */ + + /* /\* Close HDF5 resources associated with dims. *\/ */ + /* for (i = 0; i < ncindexsize(grp->dim); i++) */ + /* { */ + /* dim = (NC_DIM_INFO_T *)ncindexith(grp->dim, i); */ + /* assert(dim); */ + + /* /\* If this is a dim without a coordinate variable, then close */ + /* * the HDF5 DIM_WITHOUT_VARIABLE dataset associated with this */ + /* * dim. *\/ */ + /* if (dim->hdf_dimscaleid && H5Dclose(dim->hdf_dimscaleid) < 0) */ + /* return NC_EHDFERR; */ + /* } */ + + /* /\* Close HDF5 resources associated with types. Set values to 0 */ + /* * after closing types. Because of type reference counters, these */ + /* * closes can be called multiple times. *\/ */ + /* for (i = 0; i < ncindexsize(grp->type); i++) */ + /* { */ + /* NC_TYPE_INFO_T *type = (NC_TYPE_INFO_T *)ncindexith(grp->type, i); */ + /* assert(type); */ + + /* /\* Close any open user-defined HDF5 typeids. *\/ */ + /* if (type->hdf_typeid && H5Tclose(type->hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ + /* type->hdf_typeid = 0; */ + /* if (type->native_hdf_typeid && H5Tclose(type->native_hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ + /* type->native_hdf_typeid = 0; */ + + /* /\* Class-specific cleanup. Only enums and vlens have HDF5 */ + /* * resources to close. *\/ */ + /* switch (type->nc_type_class) */ + /* { */ + /* case NC_ENUM: */ + /* if (type->u.e.base_hdf_typeid && H5Tclose(type->u.e.base_hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ + /* type->u.e.base_hdf_typeid = 0; */ + /* break; */ + + /* case NC_VLEN: */ + /* if (type->u.v.base_hdf_typeid && H5Tclose(type->u.v.base_hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ + /* type->u.v.base_hdf_typeid = 0; */ + /* break; */ + + /* default: /\* Do nothing. *\/ */ + /* break; */ + /* } */ + /* } */ + + /* /\* Close the HDF5 group. *\/ */ + /* LOG((4, "%s: closing group %s", __func__, grp->hdr.name)); */ + /* if (grp->hdf_grpid && H5Gclose(grp->hdf_grpid) < 0) */ + /* return NC_EHDFERR; */ + + return NC_NOERR; +} + #ifdef LOGGING /* We will need to check against nc log level from nc4internal.c. */ extern int nc_log_level; From 5cbde66781c062eb7125906c9b27348c99d85c23 Mon Sep 17 00:00:00 2001 From: Ed Hartnett Date: Thu, 18 Oct 2018 03:43:53 -0600 Subject: [PATCH 05/30] now closing att HDF5 typeid in HDF5 close code --- libhdf5/hdf5internal.c | 18 +++++++++--------- libsrc4/nc4internal.c | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/libhdf5/hdf5internal.c b/libhdf5/hdf5internal.c index 4a09cd18f..e66387e95 100644 --- a/libhdf5/hdf5internal.c +++ b/libhdf5/hdf5internal.c @@ -495,16 +495,16 @@ nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp) i)))) return retval; - /* /\* Close HDF5 resources associated with attributes. *\/ */ - /* for (a = 0; a < ncindexsize(grp->att); a++) */ - /* { */ - /* att = (NC_ATT_INFO_T *)ncindexith(grp->att, a); */ - /* assert(att); */ + /* Close HDF5 resources associated with attributes. */ + for (a = 0; a < ncindexsize(grp->att); a++) + { + att = (NC_ATT_INFO_T *)ncindexith(grp->att, a); + assert(att); - /* /\* Close the HDF5 typeid. *\/ */ - /* if (att->native_hdf_typeid && H5Tclose(att->native_hdf_typeid) < 0) */ - /* return NC_EHDFERR; */ - /* } */ + /* Close the HDF5 typeid. */ + if (att->native_hdf_typeid && H5Tclose(att->native_hdf_typeid) < 0) + return NC_EHDFERR; + } /* /\* Close HDF5 resources associated with vars. *\/ */ /* for (i = 0; i < ncindexsize(grp->vars); i++) */ diff --git a/libsrc4/nc4internal.c b/libsrc4/nc4internal.c index 16f61a1ef..2066c109e 100644 --- a/libsrc4/nc4internal.c +++ b/libsrc4/nc4internal.c @@ -1412,9 +1412,9 @@ nc4_att_free(NC_ATT_INFO_T *att) att->hdr.name = NULL; } - /* Close the HDF5 typeid. */ - if (att->native_hdf_typeid && H5Tclose(att->native_hdf_typeid) < 0) - return NC_EHDFERR; + /* /\* Close the HDF5 typeid. *\/ */ + /* if (att->native_hdf_typeid && H5Tclose(att->native_hdf_typeid) < 0) */ + /* return NC_EHDFERR; */ /* If this is a string array attribute, delete all members of the * string array, then delete the array of pointers to strings. (The From e40eb2e9504ddf162ebac0f71f77a66632635636 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Fri, 19 Oct 2018 11:11:36 -0600 Subject: [PATCH 06/30] switch --- libdispatch/nclist.c | 10 +++++++--- libhdf5/hdf5file.c | 1 + libhdf5/nc4info.c | 37 +++++++++++++++++++++++-------------- libsrc/memio.c | 22 +++++++++++++++------- nc_test/Make0 | 13 ++++++++++--- nc_test/Makefile.am | 10 ++++++++++ nc_test4/Makefile.am | 14 +++++++++++++- 7 files changed, 79 insertions(+), 28 deletions(-) diff --git a/libdispatch/nclist.c b/libdispatch/nclist.c index 4890119d0..679c1b174 100644 --- a/libdispatch/nclist.c +++ b/libdispatch/nclist.c @@ -53,12 +53,16 @@ Free a list and its contents int nclistfreeall(NClist* l) { - size_t i; + size_t i,len; + void** content = NULL; if(l == NULL) return TRUE; - for(i=0;ilength;i++) { - void* value = l->content[i]; + len = l->length; + content = nclistextract(l); + for(i=0;i