fix memory leak: free g_tid in Driver::runTest

[sahitya-chandra]

Summary

Fixes a memory leak in src/Driver/Driver.cpp where g_tid was allocated with malloc but never freed.

Changes

• Free g_tid after all worker threads are joined at the end of runTest().
• Free g_tid before throwing when pthread_join fails so the allocation is not leaked on the error path.
• Set g_tid = NULL after freeing to avoid use-after-free.
• Add #include <cstdlib> for free().

[markwkm]

This is destroying all the handles to all the threads if there is an issues joining one of them right?

[sahitya-chandra]

Yes sir, free(g_tid) frees the whole array, so we lose every pthread_t in one go, including threads we haven’t joined yet.

That’s intentional on this path: pthread_join failing is treated as fatal and we throw immediately afterward, so the normal driver run doesn’t continue. We still free(g_tid) so we don’t leak if the exception is ever caught or for static analysis.

I’ve added a short comment above that free in the error branch documenting this tradeoff.

[markwkm]

This feels a little heavy handed if threads are still running, or rather maybe what we should really do based should be based on the situation. Needs some more thought...

[sahitya-chandra]

I think what went wrong with my current approach is that if we free(g_tid) as soon as one pthread_join fails, we throw away every handle in one shot. Other threads can still be running or at least still need a join, so that’s heavy-handed (as pointed out by you also): we’ve lost the ability to join them even though we might still want to try.

[markwkm]

@sahitya-chandra Since you are vying for a Google Summer of Code mentorship, I'm going to respond in an unconventional way. What I was looking for is discussion that shows understanding of what the program is meant to be doing and what it should be doing instead. Your code pushes after each of my comments indicates you think there is a binary response to my questions.

[sahitya-chandra]

You’re right @markwkm sir, I treated your notes as “fix the code” instead of stepping back and explaining what it does, first

What this code is doing: runTest starts the data-maintenance thread plus iUsers customer threads and stores their pthread_t values in g_tid. The join loop is meant to block until all of those threads have finished, so we don’t return from the test while work is still running.

[sahitya-chandra]

I think what went wrong with my current approach is that if we free(g_tid) as soon as one pthread_join fails, we throw away every handle in one shot. Other threads can still be running or at least still need a join, so that’s heavy-handed (as pointed out by you also): we’ve lost the ability to join them even though we might still want to try.

I think the better approach would be to attempt pthread_join on every slot (0 … iUsers) even if an earlier join fails, then free(g_tid) once, and only then throw if any join failed. That keeps handles valid until we’ve tried every join, avoids leaking g_tid, and reports failure after we’ve done what we can.