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.

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

DllMain leaks a thread handle #5

Closed
CookiePLMonster opened this issue Feb 24, 2023 · 4 comments
Closed

DllMain leaks a thread handle #5

CookiePLMonster opened this issue Feb 24, 2023 · 4 comments

Comments

@CookiePLMonster
Copy link

For completeness, the handle returned by CreateThread should be closed, so the thread doesn't stick around. It's not an issue at this scope at all, but since it can be fixed with one success check and one call, no reason not to do it either.

@Lyall
Copy link
Owner

Lyall commented Feb 24, 2023

Totally worth fixing as you say. Am I right in thinking something as simple as this would be sufficient?

BOOL APIENTRY DllMain( HMODULE hModule,
                       DWORD  ul_reason_for_call,
                       LPVOID lpReserved
                     )
{
    switch (ul_reason_for_call)
    {
    case DLL_PROCESS_ATTACH:
    {
        CreateThread(NULL, 0, Main, 0, NULL, 0);

        if (Main)
        {
            CloseHandle(Main);
        }
    }
    case DLL_THREAD_ATTACH:
    case DLL_THREAD_DETACH:
    case DLL_PROCESS_DETACH:
        break;
    }
    return TRUE;
}

You'll have to forgive me, I'm still quite the novice when it comes to C++.

@CookiePLMonster
Copy link
Author

CookiePLMonster commented Feb 24, 2023

Main is not a handle so it needs to be something like

        HANDLE handle = CreateThread(NULL, 0, Main, 0, NULL, 0);

        if (handle != nullptr)
        {
            CloseHandle(handle);
        }

@Lyall
Copy link
Owner

Lyall commented Feb 24, 2023

Awesome, I'll add this to the next version.

    case DLL_PROCESS_ATTACH:
    {
        HANDLE mainHandle = CreateThread(NULL, 0, Main, 0, NULL, 0);

        if (mainHandle != nullptr)
        {
            CloseHandle(mainHandle);
        }
    }

@Lyall
Copy link
Owner

Lyall commented Feb 25, 2023

That's it applied in the latest release. Thank you again for the report and letting me know how to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants