0

I need one clarification related to the Windows Threads (WEC7). Consider the following sample code. I would like to know whether any memory leak in the code.

In the code snippet MyThread1 creates memory in the heap and passed to MyThread2 and the allocated memory is cleared there.

DWORD WINAPI MyThread2(LPVOID lpVoid)
{
    int* b = (int*)lpVoid;
    int c = *b;
    free(lpVoid);
    return 0;
}


DWORD WINAPI MyThread1(LPVOID lpVoid)
{
    int count =100;
    while(count)
    {
        int* a = NULL;
        a= (int*)malloc(sizeof(int));
        *a =  count;
        CloseHandle(CreateThread(NULL, 0, MyThread2,(LPVOID)a, 0, NULL));
        count --;
    }
    return 0;
}
int main()
{


    CreateThread(NULL, 0, MyThread1,NULL, 0, NULL);
    // wait here until MyThread1 exits.

    return 0;
}
akhil
  • 732
  • 3
  • 13
  • 1
    What is `a` in `main()`? – Remy Lebeau Mar 25 '14 at 08:03
  • sorry, I have edited the code – akhil Mar 25 '14 at 08:07
  • could you maybe describe the problem you are trying to solve also? – AndersK Mar 25 '14 at 08:09
  • While this isn't necessarily a problem here, as a general rule, you should probably use `_beginthreadex` instead of `CreateThread` if you're going to rely on the C runtime's functionalities in the spawned thread (which you are right now, since you're using `free` there). – user541686 Mar 25 '14 at 08:24
  • @Mehrdad That restriction was lifted a long time ago. `CreateThread` is fine with modern runtimes. – David Heffernan Mar 25 '14 at 09:17
  • @DavidHeffernan: Are you sure? I just checked the CRT for VC++ 2013 and in the body of `_beginthreadex` I saw `/* Allocate and initialize a per-thread data structure for the to-be-created thread. */ if ((ptd = (_ptiddata)_calloc_crt(1, sizeof(struct _tiddata))) == NULL) goto error_return;` which to me seems pretty important... – user541686 Mar 25 '14 at 09:23
  • @Mehrdad http://stackoverflow.com/questions/11808525/thread-creation-the-crt-and-dlls-how-is-it-meant-to-be-done – David Heffernan Mar 25 '14 at 09:24
  • @DavidHeffernan: Do you know where Hans got the information from? I've never seen this in documentation. – user541686 Mar 25 '14 at 09:27
  • @Mehrdad Ask your self how you are going to use the thread pool if you follow the policy that you suggest – David Heffernan Mar 25 '14 at 09:27
  • @DavidHeffernan: By using `HeapAlloc` instead of `malloc`? – user541686 Mar 25 '14 at 09:29
  • @DavidHeffernan: Also, `FlsAlloc` that Hans mentions doesn't even *exist* on Windows XP, so it's quite impossible for the CRTs past VC6 to provide a callback to it for automatic cleanup. – user541686 Mar 25 '14 at 09:33
  • @Mehrdad: looking at the CRT code, it uses `FlsAlloc` if it is present, and according to Aaron's blog the DLL version of the CRT uses DllMain as well. So I think the only situation in which you might leak some per-thread memory is if you're using the static CRT on Windows XP or earlier. With XP out of support in four days and counting, I don't think this is a big issue. :-) – Harry Johnston Mar 27 '14 at 22:53

2 Answers2

2

The code you have shown will leak if CreateThread() in MyThread1() fails to create a new thread. You are not checking for that condition so MyThread1() can free the memory it allocated. Other than that, since you are allocating memory in one thread and freeing it in another thread, make sure you are using the multi-threaded version of your compiler's RTL.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
1

There's no leak of memory. The calls to malloc are matched by calls to free.

It is possible the some thread 2 instances have not started to run when thread 1 finishes and the program closes. But at that point the system reclaims memory.

You don't call CloseHandle for thread 1, but then I guess this isn't real code. In the real code you would for sure have to capture the handle for thread 1 so you can wait on it.

HANDLE hThread1 = CreateThread(NULL, 0, MyThread1,NULL, 0, NULL);
WaitForSingleObject(hThread1, INFINITE);
CloseHandle(hThread1);

Note that I've adopted your policy of eliding error checking for simplicity of exposition. Of course in real code you'd add error handling for all API calls.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490