From d934f081d22b04fd6576b21008d2bb7bf0182a72 Mon Sep 17 00:00:00 2001 From: Raymond Lu Date: Wed, 3 Jul 2002 16:06:55 -0500 Subject: [svn-r5764] Purpose: Bug Fix. Description: VL type memory leak when data is overwritten. Solution: Free heap objects holding old data. Platforms tested: Linux 2.2(eirene), IRIX 6.5(paz). --- src/H5D.c | 14 ++++++++++++-- src/H5Tconv.c | 59 +++++++++++++++++++++++++++++++++++++++-------------------- src/H5Tpkg.h | 8 ++++---- src/H5Tvlen.c | 32 ++++++++++++++++++++++++++++---- 4 files changed, 83 insertions(+), 30 deletions(-) diff --git a/src/H5D.c b/src/H5D.c index 1bd7081..d125bd5 100644 --- a/src/H5D.c +++ b/src/H5D.c @@ -1545,7 +1545,13 @@ H5D_create(H5G_entry_t *loc, const char *name, const H5T_t *type, if(IS_H5FD_MPIO(f) && dcpl_pline.nfilters > 0) HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, NULL, "Parallel I/O does not support filters yet"); #endif /*H5_HAVE_PARALLEL*/ - + + if(H5P_get(plist, H5D_CRT_FILL_TIME_NAME, &fill_time) < 0) + HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "can't retrieve fill time"); + + if(fill_time==H5D_FILL_TIME_NEVER && H5T_detect_class(type, H5T_VLEN)) + HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, NULL, "Dataset doesn't support VL datatype when fill value is not defined"); + /* Initialize the dataset object */ if(NULL == (new_dset = H5D_new(dcpl_id))) HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); @@ -2876,6 +2882,10 @@ H5D_write(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, if(H5P_get(dx_plist, H5D_XFER_BKGR_BUF_TYPE_NAME, &need_bkg)<0) HGOTO_ERROR (H5E_PLIST, H5E_CANTGET, FAIL, "Can't retrieve background buffer type"); need_bkg = MAX (tpath->cdata.need_bkg, need_bkg); + } else if(H5T_detect_class(dataset->type, H5T_VLEN)) { + /* Old data is retrieved into background buffer for VL datatype. The + * data is used later for freeing heap objects. */ + need_bkg = H5T_BKG_YES; } else { need_bkg = H5T_BKG_NO; /*never needed even if app says yes*/ } /* end else */ @@ -2887,7 +2897,7 @@ H5D_write(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, if (need_bkg && NULL==(bkg_buf=H5P_peek_voidp(dx_plist,H5D_XFER_BKGR_BUF_NAME))) { /* Allocate background buffer */ H5_CHECK_OVERFLOW((request_nelmts*dst_type_size),hsize_t,size_t); - if((bkg_buf=H5FL_BLK_ALLOC(bkgr_conv,(size_t)(request_nelmts*dst_type_size),0))==NULL) + if((bkg_buf=H5FL_BLK_ALLOC(bkgr_conv,(size_t)(request_nelmts*dst_type_size),1))==NULL) HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for background conversion"); } /* end if */ diff --git a/src/H5Tconv.c b/src/H5Tconv.c index 1c23fc3..b155384 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -2141,8 +2141,9 @@ H5T_conv_enum(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, * 2. Copy VL data from src buffer into dst buffer * 3. Convert VL data into dst representation * 4. Allocate buffer in dst heap - * 5. Write dst VL data into dst heap - * 6. Store (heap ID or pointer) and length in main dst buffer + * 5. Free heap objects storing old data + * 6. Write dst VL data into dst heap + * 7. Store (heap ID or pointer) and length in main dst buffer * * Return: Non-negative on success/Negative on failure * @@ -2157,12 +2158,17 @@ H5T_conv_enum(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, * BUF_STRIDE bytes each time; otherwise assume both source and * destination values are packed. * + * Raymond Lu, 26 June, 2002 + * Background buffer is used for freeing heap objects storing + * old data. At this moment, it only frees the first level of + * VL datatype. It doesn't handle nested VL datatypes. + * *------------------------------------------------------------------------- */ herr_t H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, size_t buf_stride, size_t bkg_stride, void *_buf, - void UNUSED *_bkg, hid_t dset_xfer_plist) + void *_bkg, hid_t dset_xfer_plist) { H5T_path_t *tpath; /* Type conversion path */ hid_t tsrc_id = -1, tdst_id = -1;/*temporary type atoms */ @@ -2171,14 +2177,16 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, hsize_t olap; /*num overlapping elements */ uint8_t *s, *sp, *d, *dp; /*source and dest traversal ptrs */ uint8_t **dptr; /*pointer to correct destination pointer*/ - size_t src_delta, dst_delta; /*source & destination stride */ + uint8_t *bg_ptr=NULL; /*background buf traversal pointer */ + uint8_t *bg=NULL; + size_t src_delta, dst_delta, bkg_delta;/*source & destination stride*/ hssize_t seq_len; /*the number of elements in the current sequence*/ size_t src_base_size, dst_base_size;/*source & destination base size*/ size_t src_size, dst_size; /*source & destination total size in bytes*/ void *conv_buf=NULL; /*temporary conversion buffer */ size_t conv_buf_size=0; /*size of conversion buffer in bytes */ - void *bkg_buf=NULL; /*temporary background buffer */ - size_t bkg_buf_size=0; /*size of background buffer in bytes */ + void *tmp_buf=NULL; /*temporary background buffer */ + size_t tmp_buf_size=0; /*size of temporary bkg buffer */ uint8_t dbuf[64],*dbuf_ptr=dbuf;/*temp destination buffer */ int direction; /*direction of traversal */ hsize_t elmtno; /*element number counter */ @@ -2231,12 +2239,16 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, if (src->size==dst->size || buf_stride>0) { olap = nelmts; sp = dp = (uint8_t*)_buf; + if(_bkg!=NULL) + bg_ptr = (uint8_t*)_bkg; direction = 1; } else if (src->size>=dst->size) { /* potentially this uses the destination buffer 1 extra * time, but its faster that floating-point calcs */ olap = ((dst->size)/(src->size-dst->size))+1; sp = dp = (uint8_t*)_buf; + if(_bkg!=NULL) + bg_ptr = (uint8_t*)_bkg; direction = 1; } else { /* potentially this uses the destination buffer 1 extra @@ -2246,6 +2258,9 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, (buf_stride ? buf_stride : src->size); dp = (uint8_t*)_buf + (nelmts-1) * (buf_stride ? buf_stride : dst->size); + if(_bkg!=NULL) + bg_ptr = (uint8_t*)_bkg + (nelmts-1) * + (bkg_stride ? bkg_stride : dst->size); direction = -1; } @@ -2254,6 +2269,7 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, */ src_delta = direction * (buf_stride ? buf_stride : src->size); dst_delta = direction * (buf_stride ? buf_stride : dst->size); + bkg_delta = direction * (bkg_stride ? bkg_stride : dst->size); /* * If the source and destination buffers overlap then use a @@ -2271,7 +2287,7 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, /* Get initial conversion buffer */ conv_buf_size=MAX(src_base_size,dst_base_size); - if ((conv_buf=H5FL_BLK_ALLOC(vlen_seq,conv_buf_size,0))==NULL) + if ((conv_buf=H5FL_BLK_ALLOC(vlen_seq,conv_buf_size,1))==NULL) HRETURN_ERROR (H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for type conversion"); /* Set up conversion path for base elements */ @@ -2284,17 +2300,19 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, } } - /* Check if we need a background buffer for this conversion */ + /* Check if we need a temporary buffer for this conversion */ if(tpath->cdata.need_bkg) { /* Set up initial background buffer */ - bkg_buf_size=MAX(src_base_size,dst_base_size); - if ((bkg_buf=H5FL_BLK_ALLOC(vlen_seq,bkg_buf_size,0))==NULL) + tmp_buf_size=MAX(src_base_size,dst_base_size); + if ((tmp_buf=H5FL_BLK_ALLOC(vlen_seq,tmp_buf_size,1))==NULL) HRETURN_ERROR (H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for type conversion"); } /* end if */ for (elmtno=0; elmtnou.vlen.getlen))(src->u.vlen.f,s); @@ -2317,25 +2335,24 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, HRETURN_ERROR(H5E_DATATYPE, H5E_READERROR, FAIL, "can't read VL data"); - /* Check if background buffer is large enough, resize if necessary */ + /* Check if temporary buffer is large enough, resize if necessary */ /* (Chain off the conversion buffer size) */ - if(tpath->cdata.need_bkg && bkg_buf_sizecdata.need_bkg && tmp_buf_sizeu.vlen.write))(dset_xfer_plist,dst->u.vlen.f,d,conv_buf, - (hsize_t)seq_len,(hsize_t)dst_base_size)<0) + if((*(dst->u.vlen.write))(dset_xfer_plist,dst->u.vlen.f,d,conv_buf, bg, (hsize_t)seq_len,(hsize_t)dst_base_size)<0) HRETURN_ERROR(H5E_DATATYPE, H5E_WRITEERROR, FAIL, "can't write VL data"); @@ -2347,6 +2364,8 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, if (d==dbuf) HDmemcpy (dp, d, dst->size); sp += src_delta; dp += dst_delta; + if(bg_ptr!=NULL) + bg_ptr += bkg_delta; /* switch destination pointer around when the olap gets to 0 */ if(--olap==0) { @@ -2361,8 +2380,8 @@ H5T_conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, hsize_t nelmts, H5FL_BLK_FREE(vlen_seq,conv_buf); /* Release the background buffer, if we have one */ - if(bkg_buf!=NULL) - H5FL_BLK_FREE(vlen_seq,bkg_buf); + if(tmp_buf!=NULL) + H5FL_BLK_FREE(vlen_seq,tmp_buf); /* Release the temporary datatype IDs used */ if (tsrc_id >= 0) diff --git a/src/H5Tpkg.h b/src/H5Tpkg.h index 9be4778..f322641 100644 --- a/src/H5Tpkg.h +++ b/src/H5Tpkg.h @@ -92,7 +92,7 @@ typedef struct H5T_enum_t { /* VL function pointers */ typedef hssize_t (*H5T_vlen_getlenfunc_t)(H5F_t *f, void *vl_addr); typedef herr_t (*H5T_vlen_readfunc_t)(H5F_t *f, void *vl_addr, void *buf, size_t len); -typedef herr_t (*H5T_vlen_writefunc_t)(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *buf, hsize_t seq_len, hsize_t base_size); +typedef herr_t (*H5T_vlen_writefunc_t)(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *buf, void *bg_addr, hsize_t seq_len, hsize_t base_size); /* A VL datatype */ typedef struct H5T_vlen_t { @@ -759,13 +759,13 @@ __DLL__ htri_t H5T_bit_inc(uint8_t *buf, size_t start, size_t size); /* VL functions */ __DLL__ hssize_t H5T_vlen_seq_mem_getlen(H5F_t *f, void *vl_addr); __DLL__ herr_t H5T_vlen_seq_mem_read(H5F_t *f, void *vl_addr, void *_buf, size_t len); -__DLL__ herr_t H5T_vlen_seq_mem_write(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *_buf, hsize_t seq_len, hsize_t base_size); +__DLL__ herr_t H5T_vlen_seq_mem_write(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *_buf, void *bg_addr, hsize_t seq_len, hsize_t base_size); __DLL__ hssize_t H5T_vlen_str_mem_getlen(H5F_t *f, void *vl_addr); __DLL__ herr_t H5T_vlen_str_mem_read(H5F_t *f, void *vl_addr, void *_buf, size_t len); -__DLL__ herr_t H5T_vlen_str_mem_write(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *_buf, hsize_t seq_len, hsize_t base_size); +__DLL__ herr_t H5T_vlen_str_mem_write(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *_buf, void *bg_addr, hsize_t seq_len, hsize_t base_size); __DLL__ hssize_t H5T_vlen_disk_getlen(H5F_t *f, void *vl_addr); __DLL__ herr_t H5T_vlen_disk_read(H5F_t *f, void *vl_addr, void *_buf, size_t len); -__DLL__ herr_t H5T_vlen_disk_write(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *_buf, hsize_t seq_len, hsize_t base_size); +__DLL__ herr_t H5T_vlen_disk_write(hid_t dxpl_id, H5F_t *f, void *vl_addr, void *_buf, void *bg_addr, hsize_t seq_len, hsize_t base_size); /* Array functions */ __DLL__ H5T_t * H5T_array_create(H5T_t *base, int ndims, diff --git a/src/H5Tvlen.c b/src/H5Tvlen.c index 24a9a1e..cc38b3c 100644 --- a/src/H5Tvlen.c +++ b/src/H5Tvlen.c @@ -204,7 +204,7 @@ herr_t H5T_vlen_seq_mem_read(H5F_t UNUSED *f, void *vl_addr, void *buf, size_t l * *------------------------------------------------------------------------- */ -herr_t H5T_vlen_seq_mem_write(hid_t plist_id, H5F_t UNUSED *f, void *vl_addr, void *buf, hsize_t seq_len, hsize_t base_size) +herr_t H5T_vlen_seq_mem_write(hid_t plist_id, H5F_t UNUSED *f, void *vl_addr, void *buf, void UNUSED *bg_addr, hsize_t seq_len, hsize_t base_size) { H5MM_allocate_t alloc_func; /* Vlen allocation function */ void *alloc_info; /* Vlen allocation information */ @@ -332,7 +332,7 @@ herr_t H5T_vlen_str_mem_read(H5F_t UNUSED *f, void *vl_addr, void *buf, size_t l * *------------------------------------------------------------------------- */ -herr_t H5T_vlen_str_mem_write(hid_t plist_id, H5F_t UNUSED *f, void *vl_addr, void *buf, hsize_t seq_len, hsize_t base_size) +herr_t H5T_vlen_str_mem_write(hid_t plist_id, H5F_t UNUSED *f, void *vl_addr, void *buf, void UNUSED *bg_addr, hsize_t seq_len, hsize_t base_size) { H5MM_allocate_t alloc_func; /* Vlen allocation function */ void *alloc_info; /* Vlen allocation information */ @@ -464,13 +464,20 @@ herr_t H5T_vlen_disk_read(H5F_t *f, void *vl_addr, void *buf, size_t UNUSED len) * * Modifications: * + * Raymond Lu + * Thursday, June 26, 2002 + * Free heap objects storing old data. + * *------------------------------------------------------------------------- */ -herr_t H5T_vlen_disk_write(hid_t UNUSED plist_id, H5F_t *f, void *vl_addr, void *buf, hsize_t seq_len, hsize_t base_size) +herr_t H5T_vlen_disk_write(hid_t UNUSED plist_id, H5F_t *f, void *vl_addr, void *buf, void *bg_addr, hsize_t seq_len, hsize_t base_size) { - uint8_t *vl=(uint8_t *)vl_addr; /* Pointer to the user's hvl_t information */ + uint8_t *vl=(uint8_t *)vl_addr; /*Pointer to the user's hvl_t information*/ + uint8_t *bg=(uint8_t *)bg_addr; /*Pointer to the old data hvl_t */ H5HG_t hobjid; + H5HG_t bg_hobjid; size_t len; + hsize_t bg_seq_len=0; FUNC_ENTER_NOAPI(H5T_vlen_disk_write, FAIL); @@ -478,6 +485,23 @@ herr_t H5T_vlen_disk_write(hid_t UNUSED plist_id, H5F_t *f, void *vl_addr, void assert(vl); assert(buf); assert(f); + + /* Get the length of the sequence and heap object ID from background data. + * Free heap object for old data. */ + if(bg!=NULL) { + UINT32DECODE(bg, bg_seq_len); + + /* Free heap object for old data */ + if(bg_seq_len!=0) { + /* Get heap information */ + H5F_addr_decode(f, (const uint8_t **)&bg, &(bg_hobjid.addr)); + INT32DECODE(bg, bg_hobjid.idx); + /* Free heap object */ + if(H5HG_remove(f, &bg_hobjid)<0) + HRETURN_ERROR(H5E_DATATYPE, H5E_WRITEERROR, FAIL, + "Unable to remove heap object"); + } /* end if */ + } /* end if */ /* Set the length of the sequence */ H5_CHECK_OVERFLOW(seq_len,hsize_t,size_t); -- cgit v0.12