From 453a3910ab7942daa7b01e821e15853cb7a0ba23 Mon Sep 17 00:00:00 2001 From: Mike McGreevy Date: Fri, 14 May 2010 13:45:07 -0500 Subject: [svn-r18805] Purpose: Fix memory leaks Description Added a routine to free memory which addresses a memory leak when variable length strings are used as fill values. Also added some minor tweaks to the H5I 'save ID structures' mechanic. Tested: h5committest and valgrind (on jam/amani) to confirm freed memory. --- release_docs/RELEASE.txt | 2 ++ src/H5Dfill.c | 22 ++++++++++++++++++++++ src/H5I.c | 7 +++++-- src/H5Ofill.c | 1 + src/H5Tprivate.h | 1 + src/H5Tvlen.c | 41 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 2 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index c28bebf..a12b8a9 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -132,6 +132,8 @@ Bug Fixes since HDF5-1.8.4 Library ------- + - Fixed some memory leaks in VL datatype conversion when strings are + used as fill values. (MAM - 2010/05/12 - BZ# 1826) - Fixed a bug when copying objects with NULL references with the H5O_COPY_EXPAND_REFERENCE_FLAG flag set. (NAF - 2010/04/08 - 1815) - Added a mechanism to the H5I interface to save returned object ID diff --git a/src/H5Dfill.c b/src/H5Dfill.c index f418a81..1db2976 100644 --- a/src/H5Dfill.c +++ b/src/H5Dfill.c @@ -570,6 +570,7 @@ herr_t H5D_fill_refill_vl(H5D_fill_buf_info_t *fb_info, size_t nelmts, hid_t dxpl_id) { herr_t ret_value = SUCCEED; /* Return value */ + void * buf = NULL; /* Temporary fill buffer */ FUNC_ENTER_NOAPI(H5D_fill_refill_vl, FAIL) @@ -605,11 +606,32 @@ H5D_fill_refill_vl(H5D_fill_buf_info_t *fb_info, size_t nelmts, hid_t dxpl_id) if(H5T_path_bkg(fb_info->mem_to_dset_tpath)) HDmemset(fb_info->bkg_buf, 0, fb_info->bkg_buf_size); + /* Make a copy of the fill buffer so we can free dynamic elements after conversion */ + if(fb_info->fill_alloc_func) + buf = fb_info->fill_alloc_func(fb_info->fill_buf_size, fb_info->fill_alloc_info); + else + buf = H5FL_BLK_MALLOC(non_zero_fill, fb_info->fill_buf_size); + HDmemcpy(buf, fb_info->fill_buf, fb_info->fill_buf_size); + /* Type convert the dataset buffer, to copy any VL components */ if(H5T_convert(fb_info->mem_to_dset_tpath, fb_info->mem_tid, fb_info->file_tid, nelmts, (size_t)0, (size_t)0, fb_info->fill_buf, fb_info->bkg_buf, dxpl_id) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTCONVERT, FAIL, "data type conversion failed") done: + if (buf) { + /* Free dynamically allocated VL elements in fill buffer */ + if (fb_info->fill->type) + H5T_vlen_reclaim_elmt(buf, fb_info->fill->type, dxpl_id); + else + H5T_vlen_reclaim_elmt(buf, fb_info->mem_type, dxpl_id); + + /* Free temporary fill buffer */ + if(fb_info->fill_free_func) + fb_info->fill_free_func(buf, fb_info->fill_free_info); + else + buf = H5FL_BLK_FREE(non_zero_fill, buf); + } /* end if */ + FUNC_LEAVE_NOAPI(ret_value) } /* end H5D_fill_refill_vl() */ diff --git a/src/H5I.c b/src/H5I.c index 3e6965f..89bed25 100644 --- a/src/H5I.c +++ b/src/H5I.c @@ -362,8 +362,10 @@ H5I_register_type(H5I_type_t type_id, size_t hash_size, unsigned reserved, HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, H5I_BADID, "memory allocation failed") /* Don't re-use IDs for property lists, as this causes problems - * with some virtual file drivers. */ - if (type_id == H5I_GENPROP_LST) + * with some virtual file drivers. Also, open datatypes are not + * getting reduced to zero before file close in some situations, + * resulting in memory leak, so skip them for now as well. */ + if (type_id == H5I_GENPROP_LST || type_id == H5I_DATATYPE) type_ptr->reuse_ids = FALSE; else type_ptr->reuse_ids = TRUE; @@ -1355,6 +1357,7 @@ H5I_remove(hid_t id) tmp_id_ptr = type_ptr->next_id_ptr->next; (void)H5FL_FREE(H5I_id_info_t, type_ptr->next_id_ptr); type_ptr->next_id_ptr = tmp_id_ptr; + type_ptr->free_count--; } /* end while */ diff --git a/src/H5Ofill.c b/src/H5Ofill.c index ffea9e6..ebe1eb5 100644 --- a/src/H5Ofill.c +++ b/src/H5Ofill.c @@ -965,6 +965,7 @@ H5O_fill_convert(H5O_fill_t *fill, H5T_t *dset_type, hbool_t *fill_changed, hid_ /* Update the fill message */ if(buf != fill->buf) { + H5T_vlen_reclaim_elmt(fill->buf, fill->type, dxpl_id); H5MM_xfree(fill->buf); fill->buf = buf; } /* end if */ diff --git a/src/H5Tprivate.h b/src/H5Tprivate.h index afa6ceb..c70eea0 100644 --- a/src/H5Tprivate.h +++ b/src/H5Tprivate.h @@ -129,6 +129,7 @@ H5_DLL herr_t H5T_convert(H5T_path_t *tpath, hid_t src_id, hid_t dst_id, size_t nelmts, size_t buf_stride, size_t bkg_stride, void *buf, void *bkg, hid_t dset_xfer_plist); H5_DLL herr_t H5T_vlen_reclaim(void *elem, hid_t type_id, unsigned ndim, const hsize_t *point, void *_op_data); +H5_DLL herr_t H5T_vlen_reclaim_elmt(void *elem, H5T_t *dt, hid_t dxpl_id); H5_DLL herr_t H5T_vlen_get_alloc_info(hid_t dxpl_id, H5T_vlen_alloc_info_t **vl_alloc_info); H5_DLL htri_t H5T_set_loc(H5T_t *dt, H5F_t *f, H5T_loc_t loc); H5_DLL htri_t H5T_is_sensible(const H5T_t *dt); diff --git a/src/H5Tvlen.c b/src/H5Tvlen.c index 8a6ee05..95f4086 100644 --- a/src/H5Tvlen.c +++ b/src/H5Tvlen.c @@ -1303,3 +1303,44 @@ done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5T_vlen_get_alloc_info() */ + +/*------------------------------------------------------------------------- + * Function: H5T_vlen_reclaim_elmt + * + * Purpose: Alternative method to reclaim any VL data for a buffer element. + * + * Use this function when the datatype is already available, but + * the allocation info is needed from the dxpl_id before jumping + * into recursion. + * + * Return: Non-negative on success/Negative on failure + * + * Programmer: Mike McGreevy + * May 11, 2010 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +herr_t +H5T_vlen_reclaim_elmt(void *elem, H5T_t *dt, hid_t dxpl_id) +{ + H5T_vlen_alloc_info_t _vl_alloc_info; /* VL allocation info buffer */ + H5T_vlen_alloc_info_t *vl_alloc_info = &_vl_alloc_info; /* VL allocation info */ + herr_t ret_value = SUCCEED; /* return value */ + + HDassert(dt); + HDassert(elem); + + FUNC_ENTER_NOAPI(H5T_vlen_reclaim_elmt, FAIL) + + /* Get VL allocation info */ + if (H5T_vlen_get_alloc_info(dxpl_id, &vl_alloc_info) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTGET, FAIL, "unable to retrieve VL allocation info") + + /* Recurse on buffer to free dynamic fields */ + ret_value = H5T_vlen_reclaim_recurse(elem,dt,vl_alloc_info->free_func,vl_alloc_info->free_info); + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* H5T_vlen_reclaim_elmt */ -- cgit v0.12