From 2e2ded61816c5dfe22bbe2825b4fc3ede8bbc434 Mon Sep 17 00:00:00 2001 From: Dana Robinson <43805+derobins@users.noreply.github.com> Date: Wed, 25 Nov 2020 19:53:48 -0800 Subject: Minor refactoring in tid.c (#127) --- test/tid.c | 376 +++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 239 insertions(+), 137 deletions(-) diff --git a/test/tid.c b/test/tid.c index 27135fc..fe458dd 100644 --- a/test/tid.c +++ b/test/tid.c @@ -569,197 +569,299 @@ out: /* There was a rare bug where, if an id free callback being called by * H5I_clear_type() removed another id in that type, a segfault could occur. * This test tests for that error (and freeing ids "out of order" within - * H5Iclear_type() in general). */ + * H5Iclear_type() in general). + * + * NB: RCT = "remove clear type" + */ + /* Macro definitions */ -#define TEST_RCT_MAX_NOBJS 25 -#define TEST_RCT_MIN_NOBJS 5 -#define TEST_RCT_NITER 50 +#define RCT_MAX_NOBJS 25 /* Maximum number of objects in the list */ +#define RCT_MIN_NOBJS 5 +#define RCT_NITER 50 /* Number of times we cycle through object creation and deletion */ + +/* Structure to hold the master list of objects */ +typedef struct rct_obj_list_t { + + /* Pointer to the objects */ + struct rct_obj_t *objects; + + /* The number of objects in the list */ + long count; -/* Structure to hold the list of objects */ -typedef struct { - struct test_rct_obj_t *list; /* List of objects */ - long nobjs; /* Number of objects in list */ - long nobjs_rem; /* Number of objects in list that have not been freed */ -} test_rct_list_t; + /* The number of objects in the list that have not been freed */ + long remaining; +} rct_obj_list_t; /* Structure for an object */ -typedef struct test_rct_obj_t { - hid_t id; /* ID for this object */ - int nfrees; /* Number of times this object has been freed */ - hbool_t freeing; /* Whether we are currently freeing this object directly (through H5Idec_ref()) */ - test_rct_list_t *obj_list; /* List of all objects */ -} test_rct_obj_t; - -/* Free callback */ +typedef struct rct_obj_t { + /* The ID for this object */ + hid_t id; + + /* The number of times this object has been freed */ + int nfrees; + + /* Whether we are currently freeing this object directly + * through H5Idec_ref(). + */ + hbool_t freeing; + + /* Pointer to the master list of all objects */ + rct_obj_list_t *list; +} rct_obj_t; + +/* Free callback passed to H5Iclear_type() + * + * When invoked on a closing object, frees a random unfreed ID in the + * master list of objects. + */ static herr_t -test_rct_free(void *_obj) +rct_free_cb(void *_obj) { - test_rct_obj_t *obj = (test_rct_obj_t *)_obj; - long rem_idx, i; - herr_t ret; /* return value */ + rct_obj_t * obj = (rct_obj_t *)_obj; + long remove_nth; + long i; + herr_t ret; /* Mark this object as freed */ obj->nfrees++; - obj->obj_list->nobjs_rem--; - - /* Check freeing and nobjs_rem */ - if (!obj->freeing && (obj->obj_list->nobjs_rem > 0)) { - /* Remove a random object from the list */ - rem_idx = HDrandom() % obj->obj_list->nobjs_rem; - - /* Scan the list, finding the rem_idx'th object that has not been - * freed */ - for (i = 0; i < obj->obj_list->nobjs; i++) - if (obj->obj_list->list[i].nfrees == 0) { - if (rem_idx == 0) + + /* Decrement the number of objects in the list that have not been freed */ + obj->list->remaining--; + + /* If this object isn't already being freed by a callback free call and + * the master object list still contains objects to free, pick another + * object and free it. + */ + if (!obj->freeing && (obj->list->remaining > 0)) { + + /* Pick a random object from the list. This is done by picking a + * random number between 0 and the # of remaining unfreed objects + * and then scanning through the list to find that nth unfreed + * object. + */ + remove_nth = HDrandom() % obj->list->remaining; + for (i = 0; i < obj->list->count; i++) + if (obj->list->objects[i].nfrees == 0) { + if (remove_nth == 0) break; else - rem_idx--; - } /* end if */ - if (i == obj->obj_list->nobjs) { + remove_nth--; + } + + /* Badness if we scanned through the list and didn't manage to + * select one to delete (the list stats were probably updated + * incorrectly). + */ + if (i == obj->list->count) { ERROR("invalid obj_list"); - goto out; - } /* end if */ - else { - /* Remove the object. Mark as "freeing" so its own callback does - * not free another object. */ - obj->obj_list->list[i].freeing = TRUE; - ret = H5Idec_ref(obj->obj_list->list[i].id); - CHECK(ret, FAIL, "H5Idec_ref"); - if (ret == FAIL) - goto out; - obj->obj_list->list[i].freeing = FALSE; - } /* end else */ - } /* end if */ - - /* Verify nobjs_rem is non-negative */ - if (obj->obj_list->nobjs_rem < 0) { - ERROR("invalid nobjs_rem"); - goto out; - } /* end if */ + goto error; + } + + /* Mark the object we're about to free so its own callback does + * not free another object. We don't want to recursively free the + * entire list when we free the first ID. + */ + obj->list->objects[i].freeing = TRUE; + + /* Decrement the reference count on the object */ + ret = H5Idec_ref(obj->list->objects[i].id); + CHECK(ret, FAIL, "H5Idec_ref"); + if (ret == FAIL) + goto error; + + /* Unset the "freeing" flag */ + obj->list->objects[i].freeing = FALSE; + } + + /* Verify the number of objects remaining in the master list is non-negative */ + if (obj->list->remaining < 0) { + ERROR("invalid number of objects remaining"); + goto error; + } return 0; -out: +error: return -1; -} /* end test_rct_free() */ +} /* end rct_free_cb() */ /* Test function */ static int test_remove_clear_type(void) { H5I_type_t obj_type; - test_rct_list_t obj_list; - test_rct_obj_t list[TEST_RCT_MAX_NOBJS]; + rct_obj_list_t obj_list; + rct_obj_t * objects = NULL; /* Convenience pointer to objects stored in master list */ + size_t list_size; long i, j; - long nobjs_found; - hsize_t nmembers; herr_t ret; /* return value */ - /* Register type */ - obj_type = H5Iregister_type((size_t)8, 0, test_rct_free); + /* Register a user-defined type with our custom ID-deleting callback */ + obj_type = H5Iregister_type((size_t)8, 0, rct_free_cb); CHECK(obj_type, H5I_BADID, "H5Iregister_type"); if (obj_type == H5I_BADID) - goto out; + goto error; + + /* Create an array to hold the objects in the master list */ + list_size = RCT_MAX_NOBJS * sizeof(rct_obj_t); + obj_list.objects = HDmalloc(list_size); + CHECK(obj_list.objects, NULL, "HDcalloc"); + if (NULL == obj_list.objects) + goto error; + + /* Set a convenience pointer to the object array */ + objects = obj_list.objects; + + for (i = 0; i < RCT_NITER; i++) { + + /* The number of members in the type, according to the HDF5 library */ + hsize_t nmembers = 1234567; + + /* The number of objects found while scanning through the object list */ + unsigned found; + + /********************* + * Build object list * + *********************/ + + HDmemset(obj_list.objects, 0, list_size); - /* Init obj_list.list */ - obj_list.list = list; - - for (i = 0; i < TEST_RCT_NITER; i++) { - /* Build object list */ - obj_list.nobjs = obj_list.nobjs_rem = - TEST_RCT_MIN_NOBJS + (HDrandom() % (long)(TEST_RCT_MAX_NOBJS - TEST_RCT_MIN_NOBJS + 1)); - for (j = 0; j < obj_list.nobjs; j++) { - list[j].nfrees = 0; - list[j].freeing = FALSE; - list[j].obj_list = &obj_list; - list[j].id = H5Iregister(obj_type, &list[j]); - CHECK(list[j].id, FAIL, "H5Iregister"); - if (list[j].id == FAIL) - goto out; + /* The number of objects used is a random number between the min and max */ + obj_list.count = obj_list.remaining = + RCT_MIN_NOBJS + (HDrandom() % (long)(RCT_MAX_NOBJS - RCT_MIN_NOBJS + 1)); + + /* Create the actual objects */ + for (j = 0; j < obj_list.count; j++) { + + /* Object setup */ + objects[j].nfrees = 0; + objects[j].freeing = FALSE; + objects[j].list = &obj_list; + + /* Register an ID for it */ + objects[j].id = H5Iregister(obj_type, &objects[j]); + CHECK(objects[j].id, FAIL, "H5Iregister"); + if (objects[j].id == FAIL) + goto error; + + /* Bump the reference count by 1 (to 2) 50% of the time */ if (HDrandom() % 2) { - ret = H5Iinc_ref(list[j].id); + ret = H5Iinc_ref(objects[j].id); CHECK(ret, FAIL, "H5Iinc_ref"); if (ret == FAIL) - goto out; - } /* end if */ - } /* end for */ + goto error; + } + } - /* Clear the type */ + /****************************************** + * Clear the type with force set to FALSE * + ******************************************/ + + /* Clear the type. Since force is FALSE, only + * IDs with a reference count of 1 will be cleared. + */ ret = H5Iclear_type(obj_type, FALSE); CHECK(ret, FAIL, "H5Iclear_type"); if (ret == FAIL) - goto out; - - /* Verify list */ - nobjs_found = 0; - for (j = 0; j < obj_list.nobjs; j++) { - if (list[j].nfrees == 0) - nobjs_found++; + goto error; + + /* Verify that the object struct fields are sane and count the + * number of unfreed objects + */ + found = 0; + for (j = 0; j < obj_list.count; j++) { + + if (objects[j].nfrees == 0) { + /* Count unfreed objects */ + found++; + } else { - VERIFY(list[j].nfrees, (long)1, "list[j].nfrees"); - if (list[j].nfrees != (long)1) - goto out; - } /* end else */ - VERIFY(list[j].freeing, FALSE, "list[j].freeing"); - if (list[j].freeing != FALSE) - goto out; - } /* end for */ - - /* Verify number of objects */ - VERIFY(obj_list.nobjs_rem, nobjs_found, "obj_list.nobjs_rem"); - if (obj_list.nobjs_rem != nobjs_found) - goto out; + /* Every freed object should have been freed exactly once */ + VERIFY(objects[j].nfrees, 1, "object freed more than once"); + if (objects[j].nfrees != 1) + goto error; + } + + /* No object should still be marked as "freeing" */ + VERIFY(objects[j].freeing, FALSE, "object marked as freeing"); + if (objects[j].freeing != FALSE) + goto error; + } + + /* Verify the number of unfreed objects we found during our scan + * matches the number stored in the list + */ + VERIFY(obj_list.remaining, found, "incorrect number of objects remaining"); + if (obj_list.remaining != found) + goto error; + + /* Make sure the HDF5 library confirms our count */ ret = H5Inmembers(obj_type, &nmembers); CHECK(ret, FAIL, "H5Inmembers"); if (ret == FAIL) - goto out; - VERIFY(nmembers, (size_t)nobjs_found, "H5Inmembers"); - if (nmembers != (size_t)nobjs_found) - goto out; + goto error; + VERIFY(nmembers, found, "The number of members remaining in the type did not match our count"); + if (nmembers != found) + goto error; + + /***************************************** + * Clear the type with force set to TRUE * + *****************************************/ - /* Clear the type with force set to TRUE */ + /* Clear the type. Since force is TRUE, all IDs will be cleared. */ ret = H5Iclear_type(obj_type, TRUE); CHECK(ret, FAIL, "H5Iclear_type"); if (ret == FAIL) - goto out; + goto error; - /* Verify list */ - for (j = 0; j < obj_list.nobjs; j++) { - VERIFY(list[j].nfrees, (long)1, "list[j].nfrees"); - if (list[j].nfrees != (long)1) - goto out; - VERIFY(list[j].freeing, FALSE, "list[j].freeing"); - if (list[j].freeing != FALSE) - goto out; - } /* end for */ - - /* Verify number of objects is 0 */ - VERIFY(obj_list.nobjs_rem, (long)0, "obj_list.nobjs_rem"); - if (obj_list.nobjs_rem != (long)0) - goto out; + /* Verify that the object struct fields are sane */ + for (j = 0; j < obj_list.count; j++) { + + /* Every object should have been freed exactly once */ + VERIFY(objects[j].nfrees, 1, "object freed more than once"); + if (objects[j].nfrees != 1) + goto error; + + /* No object should still be marked as "freeing" */ + VERIFY(objects[j].freeing, FALSE, "object marked as freeing"); + if (objects[j].freeing != FALSE) + goto error; + } + + /* Verify the number of objects is 0 */ + VERIFY(obj_list.remaining, 0, "objects remaining was not zero"); + if (obj_list.remaining != 0) + goto error; + + /* Make sure the HDF5 library confirms zero members in the type */ ret = H5Inmembers(obj_type, &nmembers); CHECK(ret, FAIL, "H5Inmembers"); if (ret == FAIL) - goto out; - VERIFY(nmembers, (size_t)0, "H5Inmembers"); - if (nmembers != (size_t)0) - goto out; - } /* end for */ + goto error; + VERIFY(nmembers, 0, "The number of members remaining in the type was not zero"); + if (nmembers != 0) + goto error; + } - /* Destroy type */ + /* Destroy the type */ ret = H5Idestroy_type(obj_type); CHECK(ret, FAIL, "H5Idestroy_type"); if (ret == FAIL) - goto out; + goto error; + + /* Free the object array */ + HDfree(obj_list.objects); return 0; -out: - /* Cleanup. For simplicity, just destroy the types and ignore errors. */ - H5E_BEGIN_TRY - H5Idestroy_type(obj_type); - H5E_END_TRY +error: + /* Cleanup. For simplicity, just destroy the types and ignore errors. */ + H5E_BEGIN_TRY { + H5Idestroy_type(obj_type); + } H5E_END_TRY + + HDfree(obj_list.objects); + return -1; } /* end test_remove_clear_type() */ -- cgit v0.12