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.

Sign up for GitHub

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

Fix memory leaks in ttsafe tests #4842

Merged
merged 1 commit into from
Sep 18, 2024
+53 −25
Merged

Fix memory leaks in ttsafe tests #4842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 22 additions & 8 deletions test/ttsafe_acreate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ tts_acreate(void)
int buffer, i;
herr_t status;

ttsafe_name_data_t *attrib_data;
ttsafe_name_data_t *attrib_data[NUM_THREADS];

char *attribute_name = NULL;

memset(attrib_data, 0, sizeof(attrib_data));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this generate size warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing cropped up in the -Werror builds. NUM_THREADS is 16, so this is 16 * 8 bytes per pointer = 128, which I think should be fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -Werror builds don't check --enable-threadsafe


/*
* Create an HDF5 file using H5F_ACC_TRUNC access, default file
Expand Down Expand Up @@ -97,12 +101,12 @@ tts_acreate(void)
* with the dataset
*/
for (i = 0; i < NUM_THREADS; i++) {
attrib_data = (ttsafe_name_data_t *)malloc(sizeof(ttsafe_name_data_t));
attrib_data->dataset = dataset;
attrib_data->datatype = datatype;
attrib_data->dataspace = dataspace;
attrib_data->current_index = i;
if (H5TS_thread_create(&threads[i], tts_acreate_thread, attrib_data) < 0)
attrib_data[i] = (ttsafe_name_data_t *)malloc(sizeof(ttsafe_name_data_t));
attrib_data[i]->dataset = dataset;
attrib_data[i]->datatype = datatype;
attrib_data[i]->dataspace = dataspace;
attrib_data[i]->current_index = i;
if (H5TS_thread_create(&threads[i], tts_acreate_thread, attrib_data[i]) < 0)
TestErrPrintf("thread # %d did not start", i);
}

Expand All @@ -112,7 +116,9 @@ tts_acreate(void)

/* verify the correctness of the test */
for (i = 0; i < NUM_THREADS; i++) {
attribute = H5Aopen(dataset, gen_name(i), H5P_DEFAULT);
attribute_name = gen_name(i);

attribute = H5Aopen(dataset, attribute_name, H5P_DEFAULT);
CHECK(attribute, H5I_INVALID_HID, "H5Aopen");

if (attribute < 0)
Expand All @@ -125,6 +131,8 @@ tts_acreate(void)
status = H5Aclose(attribute);
CHECK(status, FAIL, "H5Aclose");
}

free(attribute_name);
}

/* close remaining resources */
Expand All @@ -136,6 +144,10 @@ tts_acreate(void)
CHECK(status, FAIL, "H5Dclose");
status = H5Fclose(file);
CHECK(status, FAIL, "H5Fclose");

for (i = 0; i < NUM_THREADS; i++)
if (attrib_data[i])
free(attrib_data[i]);
} /* end tts_acreate() */

H5TS_THREAD_RETURN_TYPE
Expand Down Expand Up @@ -164,6 +176,8 @@ tts_acreate_thread(void *client_data)
status = H5Aclose(attribute);
CHECK(status, FAIL, "H5Aclose");

free(attribute_data);
free(attribute_name);
return (H5TS_thread_ret_t)0;
} /* end tts_acreate_thread() */

Expand Down
12 changes: 12 additions & 0 deletions test/ttsafe_attr_vlen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ tts_attr_vlen_thread(void H5_ATTR_UNUSED *client_data)
hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t gid = H5I_INVALID_HID; /* Group ID */
hid_t aid = H5I_INVALID_HID; /* Attribute ID */
hid_t asid = H5I_INVALID_HID; /* Dataspace ID for the attribute */
hid_t atid = H5I_INVALID_HID; /* Datatype ID for the attribute */
char *string_attr_check; /* The attribute data being read */
const char *string_attr = "2.0"; /* The expected attribute data */
Expand All @@ -144,17 +145,28 @@ tts_attr_vlen_thread(void H5_ATTR_UNUSED *client_data)
atid = H5Aget_type(aid);
CHECK(atid, H5I_INVALID_HID, "H5Aget_type");

/* Get the dataspace for the attribute */
asid = H5Aget_space(aid);
CHECK(asid, H5I_INVALID_HID, "H5Aget_space");

/* Read the attribute */
ret = H5Aread(aid, atid, &string_attr_check);
CHECK(ret, FAIL, "H5Aclose");

/* Verify the attribute data is as expected */
VERIFY_STR(string_attr_check, string_attr, "H5Aread");

/* Free the attribute data */
ret = H5Dvlen_reclaim(atid, asid, H5P_DEFAULT, &string_attr_check);
CHECK(ret, FAIL, "H5Dvlen_reclaim");

/* Close IDs */
ret = H5Aclose(aid);
CHECK(ret, FAIL, "H5Aclose");

ret = H5Sclose(asid);
CHECK(ret, FAIL, "H5Aclose");

ret = H5Gclose(gid);
CHECK(ret, FAIL, "H5Aclose");

Expand Down
36 changes: 19 additions & 17 deletions test/ttsafe_cancel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ typedef struct cleanup_struct {
hid_t dataspace;
} cancel_cleanup_t;

/* Used by tts_cancel_thread.
* Global because the thread gets cancelled and can't clean up its allocations */
cancel_cleanup_t cleanup_structure = {H5I_INVALID_HID, H5I_INVALID_HID, H5I_INVALID_HID};

pthread_t childthread;
static H5TS_barrier_t barrier;

Expand Down Expand Up @@ -94,14 +98,13 @@ tts_cancel(void)
void *
tts_cancel_thread(void H5_ATTR_UNUSED *arg)
{
hid_t dataspace = H5I_INVALID_HID;
hid_t datatype = H5I_INVALID_HID;
hid_t dataset = H5I_INVALID_HID;
int datavalue;
int buffer;
hsize_t dimsf[1]; /* dataset dimensions */
cancel_cleanup_t *cleanup_structure;
herr_t status;
hid_t dataspace = H5I_INVALID_HID;
hid_t datatype = H5I_INVALID_HID;
hid_t dataset = H5I_INVALID_HID;
int datavalue;
int buffer;
hsize_t dimsf[1]; /* dataset dimensions */
herr_t status;

/* define dataspace for dataset */
dimsf[0] = 1;
Expand All @@ -120,11 +123,10 @@ tts_cancel_thread(void H5_ATTR_UNUSED *arg)
CHECK(dataset, H5I_INVALID_HID, "H5Dcreate2");

/* If thread is cancelled, make cleanup call */
cleanup_structure = (cancel_cleanup_t *)malloc(sizeof(cancel_cleanup_t));
cleanup_structure->dataset = dataset;
cleanup_structure->datatype = datatype;
cleanup_structure->dataspace = dataspace;
pthread_cleanup_push(cancellation_cleanup, cleanup_structure);
cleanup_structure.dataset = dataset;
cleanup_structure.datatype = datatype;
cleanup_structure.dataspace = dataspace;
pthread_cleanup_push(cancellation_cleanup, &cleanup_structure);

datavalue = 1;
status = H5Dwrite(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &datavalue);
Expand Down Expand Up @@ -189,14 +191,14 @@ tts_cancel_callback(void *elem, hid_t H5_ATTR_UNUSED type_id, unsigned H5_ATTR_U
void
cancellation_cleanup(void *arg)
{
cancel_cleanup_t *cleanup_structure = (cancel_cleanup_t *)arg;
cancel_cleanup_t *_cleanup_structure = (cancel_cleanup_t *)arg;
herr_t status;

status = H5Dclose(cleanup_structure->dataset);
status = H5Dclose(_cleanup_structure->dataset);
CHECK(status, FAIL, "H5Dclose");
status = H5Tclose(cleanup_structure->datatype);
status = H5Tclose(_cleanup_structure->datatype);
CHECK(status, FAIL, "H5Tclose");
status = H5Sclose(cleanup_structure->dataspace);
status = H5Sclose(_cleanup_structure->dataspace);
CHECK(status, FAIL, "H5Sclose");
} /* end cancellation_cleanup() */

Expand Down
Loading