Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate the closing of HDF5 objects from freeing of internal metadata #1167

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
979873f
Fix provenance memory leak
DennisHeimbigner Oct 17, 2018
c90ab24
moving towards separating HDF5 file close from netcdf4 file close
edhartnett Oct 18, 2018
fa86d3c
continuing to separate hdf5/libsrc4 file close code
edhartnett Oct 18, 2018
695e295
now calling recursive function to close HDF5 objects in file
edhartnett Oct 18, 2018
5cbde66
now closing att HDF5 typeid in HDF5 close code
edhartnett Oct 18, 2018
e40eb2e
switch
DennisHeimbigner Oct 19, 2018
1851b39
Merge branch 'master' into infoleak.dmh
DennisHeimbigner Oct 19, 2018
0e9f1bb
ckp
DennisHeimbigner Oct 19, 2018
2862c01
working on getting build working
edhartnett Oct 20, 2018
05b39db
restoring valgrind dependencies
edhartnett Oct 21, 2018
76745e5
moved dependencies
edhartnett Oct 21, 2018
188c582
moved dependencies
edhartnett Oct 21, 2018
18a145f
moved dependencies
edhartnett Oct 21, 2018
d261c4a
moved dependencies
edhartnett Oct 21, 2018
01158b0
moved dependencies
edhartnett Oct 21, 2018
975eb8a
changed run_diskless2.sh
edhartnett Oct 21, 2018
234e2ad
messing with run_diskless2.sh to produce error
edhartnett Oct 22, 2018
9e171c3
getting tests working
edhartnett Oct 22, 2018
d2a5a0d
merged master
edhartnett Oct 22, 2018
958826d
merged ejh_mem_check
edhartnett Oct 22, 2018
452f75f
commented out some tests
edhartnett Oct 22, 2018
6589867
clean up var_free
edhartnett Oct 23, 2018
35cfaef
closing HDF5 objects separately
edhartnett Oct 23, 2018
c2cb337
doing HDF5 closing in libhdf5
edhartnett Oct 23, 2018
be4fe7a
clean up
edhartnett Oct 23, 2018
1d952cb
clean up
edhartnett Oct 23, 2018
73de4f6
clean up
edhartnett Oct 23, 2018
96ad72f
undid unintentional change to Makefile.am
edhartnett Oct 23, 2018
abd0bc0
undid unintentional change to Makefile.am
edhartnett Oct 23, 2018
5f36a3b
merged master
edhartnett Nov 2, 2018
0ce28eb
undid some unnecessary changes
edhartnett Nov 2, 2018
571cf6b
Fix test run_diskless2.sh with LARGE_FILE_TESTS
DennisHeimbigner Nov 3, 2018
c637692
merged Dennis' fix for largefile tests
edhartnett Nov 5, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/hdf5internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ typedef struct NC_HDF5_FILE_INFO
int rec_detach_scales(NC_GRP_INFO_T *grp, int dimid, hid_t dimscaleid);
int rec_reattach_scales(NC_GRP_INFO_T *grp, int dimid, hid_t dimscaleid);
void reportopenobjects(int log, hid_t);

int nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp);

/* Used by NC4_set_provenance */
int nc4_put_att(NC_GRP_INFO_T* grp, int varid, const char *name, nc_type file_type,
Expand Down
6 changes: 2 additions & 4 deletions include/nc4internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,8 @@ int nc4_var_list_add(NC_GRP_INFO_T* grp, const char* name, int ndims, NC_VAR_INF
int nc4_var_list_add2(NC_GRP_INFO_T* grp, const char* name, NC_VAR_INFO_T **var);
int nc4_var_set_ndims(NC_VAR_INFO_T *var, int ndims);
int nc4_var_list_del(NC_GRP_INFO_T* grp, NC_VAR_INFO_T *var);
int nc4_var_free(NC_VAR_INFO_T *var);
int nc4_dim_list_add(NC_GRP_INFO_T* grp, const char* name, size_t len, int assignedid, NC_DIM_INFO_T **dim);
int nc4_dim_list_del(NC_GRP_INFO_T* grp, NC_DIM_INFO_T *dim);
int nc4_dim_free(NC_DIM_INFO_T *dim);
int nc4_type_new(NC_GRP_INFO_T *grp, size_t size, const char *name, int assignedid, NC_TYPE_INFO_T **type);
int nc4_type_list_add(NC_GRP_INFO_T *grp, size_t size, const char *name, NC_TYPE_INFO_T **type);
int nc4_type_list_del(NC_GRP_INFO_T* grp, NC_TYPE_INFO_T *type);
Expand All @@ -389,7 +387,6 @@ int nc4_field_list_add(NC_TYPE_INFO_T* parent, const char *name,
nc_type xtype, int ndims, const int *dim_sizesp);
int nc4_att_list_add(NCindex* list, const char* name, NC_ATT_INFO_T **att);
int nc4_att_list_del(NCindex* list, NC_ATT_INFO_T *att);
int nc4_att_free(NC_ATT_INFO_T *att);
int nc4_grp_list_add(NC_FILE_INFO_T *h5, NC_GRP_INFO_T *parent, char *name, NC_GRP_INFO_T **grp);
int nc4_build_root_grp(NC_FILE_INFO_T* h5);
int nc4_rec_grp_del(NC_GRP_INFO_T *grp);
Expand All @@ -410,7 +407,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/finalization */
extern int nc4_hdf5_initialized;
Expand Down
2 changes: 1 addition & 1 deletion libhdf5/hdf5create.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ nc4_create_file(const char *path, int cmode, size_t initialsz,
#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;
}

Expand Down
63 changes: 50 additions & 13 deletions libhdf5/hdf5file.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,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;
Expand All @@ -174,16 +174,6 @@ 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;

/* 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. */
if ((retval = nc4_rec_grp_del(h5->root_grp)))
Expand Down Expand Up @@ -252,6 +242,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.
Expand Down Expand Up @@ -515,7 +552,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. */
Expand Down Expand Up @@ -564,7 +601,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;
Expand Down
123 changes: 123 additions & 0 deletions libhdf5/hdf5internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,129 @@ nc4_reform_coord_var(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var, NC_DIM_INFO_T *dim)
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;
Expand Down
2 changes: 1 addition & 1 deletion libhdf5/hdf5open.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc)
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;
}

Expand Down
Loading