1

I am having an issue creating a thread inside of another thread. Normally I would be able to do this, but the reason for this issue is because I've Incremented Reference Count of the DLL which starts these threads. I need to start multiple threads inside this DLL. How can I get around this and be able to issue multiple CreateThread()'s when needed in my project without experiencing problems because of the Incremented Reference Count in my DLL?

Here is the function I've written to Increment Reference Count in my DLL file:

BOOL IncrementReference( HMODULE hModule )
{
    if ( hModule == NULL )
        return FALSE;

    TCHAR ModulePath[ MAX_PATH + 1 ];
    if ( GetModuleFileName( hModule , ModulePath , MAX_PATH ) == 0 )
        return FALSE;

    if ( LoadLibrary( ModulePath ) == NULL )
        return FALSE;

    return TRUE;
}

As requested, here is a PoC program to recreate the issue I am facing. I am really hoping this will help you guys point me to a solution. Also, take note, the DLL is being unloading due to conditions in the application which I am targeting (hooks that are already set in that application), so Incrementing the Reference Count is required for my thread to run in the first place.

Also, I can't run more than one operation in the main thread as it has its own functionality to take care of and another thread is required on the side to take care of something else. They must also run simultaneously, hence I need to fix this issue of making more than one thread in an Incremented DLL.

// dllmain.cpp : Defines the entry point for the DLL application.
#pragma comment( linker , "/Entry:DllMain" )
#include <Windows.h>
#include <process.h>

UINT CALLBACK SecondThread( PVOID pParam )
{
    MessageBox( NULL , __FUNCTION__ , "Which Thread?" , 0 );
    return 0;
}

UINT CALLBACK FirstThread( PVOID pParam )
{
  MessageBox( NULL , __FUNCTION__ , "Which Thread?" , 0 );
  _beginthreadex(0, 0, &SecondThread, 0, 0, 0);
  return 0;
}

BOOL IncrementReference( HMODULE hModule )
{
    if ( hModule == NULL )
        return FALSE;

    TCHAR ModulePath[ MAX_PATH + 1 ];
    if ( GetModuleFileName( hModule , ModulePath , MAX_PATH ) == 0 )
        return FALSE;

    if ( LoadLibrary( ModulePath ) == NULL )
        return FALSE;

    return TRUE;
}

BOOL APIENTRY DllMain( HMODULE hModule,
                       DWORD  ul_reason_for_call,
                       LPVOID lpReserved
                     )
{
    switch (ul_reason_for_call)
    {
        case DLL_PROCESS_ATTACH:
        {
            if (IncrementReference(0))    
                _beginthreadex(0, 0, &FirstThread, 0, 0, 0);
        }
        break;
    }
    return TRUE;
}

As you can see, the code never executes the SecondThread function. The question is, why? And what can be done to fix it?

Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
  • Would it not be better to have one thread that manages the other threads, and thus not have to load the module more than once in the first place? – Mats Petersson Nov 29 '15 at 00:11
  • 1
    You don't say what the problem actually is, and the posted code contains nothing about threads. – dxiv Nov 29 '15 at 02:55
  • Better than your last try, but we really do need a [full MCVE](http://stackoverflow.com/help/mcve). That is, a *complete* program - one we can build and run ourselves to see what's going wrong. It must also be as short as possible, so remove any code that isn't necessary to demonstrate the problem. – Harry Johnston Nov 29 '15 at 04:00
  • 1
    As is, my best guess: you're calling IncrementReference() from DllMain(). If so, the call to LoadLibrary is against the rules. Perhaps the loader lock has broken, that would cause all new threads to deadlock. PS: you should really be trying to fix the underlying problem, i.e., why is the application unloading your DLL prematurely, rather than trying to mess with the instance count. – Harry Johnston Nov 29 '15 at 04:05
  • 'Inside another thread' doesn't actually mean anything. All code is executed by a thread, so any time yo create another thread you are already executing as a thread. – user207421 Nov 29 '15 at 06:06
  • There is no containment relation or parent/child relation between threads either. – Ulrich Eckhardt Nov 29 '15 at 09:37
  • I've just realized that your call to IncrementReference() isn't doing anything, since you're passing NULL. So if the MCVE as posted actually does demonstrate the problem you're talking about, it has nothing to do with the DLL reference. (Also the MCVE as posted never tries to launch FirstThread, since IncrementReference returns FALSE.) – Harry Johnston Nov 29 '15 at 20:57
  • Robert, can we get some feedback on this please? Do you still have a problem after making the changes I suggested in my answer? – Harry Johnston Dec 03 '15 at 19:45
  • @HarryJohnston I managed to "bypass" the deadlock from LoadLibrary using VirtualLock(), reallocating the PE base entry and remapping the DLL from the previous image after its reloaded by LoadLibrary then calling the CreateThread, from the new entry point which is located inside the thread which was loaded in the last PE image. I managed to fix the issue by applying that routine. – Robert Frost Dec 09 '15 at 20:35

3 Answers3

2
   #pragma comment( linker , "/Entry:DllMain" )

That was a very bad idea, the proper entrypoint for a DLL is not in fact DllMain(). You have to keep in mind that WinMain and DllMain are just place-holder names. A way for Microsoft to document the relevance of executable file entrypoints. By convention you use those same names in your program, everybody will understand what they do.

But there's a very important additional detail in a C or C++ program, the CRT (C runtime library) needs to be initialized first. Before you can run any code that might make CRT function calls. Like _beginthreadex().

In other words, the default /ENTRY linker option is not DllMain(). The real entrypoint of a DLL is _DllMainCRTStartup(). A function inside the CRT that takes care of the required initialization, then calls DllMain(). If you wrote one in your program then that's the one that runs. If you didn't then a dummy one in the CRT gets linked.

All bets are off when you make CRT function calls and the CRT wasn't initialized. You must remove that #pragma so the linker will use the correct entrypoint.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • In fact, `_DllMainCRTStartup()` doesn't just initialize the CRT, but also ensures that certain language guarantees are met. One common such guarantee is that objects with static storage duration are initialized before an application runs. `_DllMainCRTStartup()` initializes those objects. – IInspectable Nov 29 '15 at 17:59
0

According to MSDN you schould neither call LoadLibrary nor CreateThread inside DllMain - your code does both!

RoMa
  • 799
  • 1
  • 8
  • 25
  • 1
    Okay, but let's say for my scenario I've already done this. How can I fix the issue in order to load the `SecondThread` function? Let's just put MSDN aside as we're programmers, and our goal is to fix issues which are not in the clear. – Robert Frost Nov 29 '15 at 06:02
  • The only solution that i know of that prevents deadlocks during DllMain processing, is to take the functunality that creates the deadlock (such as creating threads or loading DLLs) out of DllMain and put it into a seperate exported function, that has to be called explicitly by the client of the DLL. – RoMa Nov 29 '15 at 06:21
  • @RobertFrost the link @RoMa posted states pretty clearly that `DllMain is called while the loader-lock is held. ...You cannot call any function in DllMain that directly or indirectly tries to acquire the loader lock. ...You should never ... call LoadLibrary or LoadLibraryEx (either directly or indirectly). This can cause a deadlock or a crash.` This is not any gray area, it's cut-and-dried. What you call "the issue" is not fixable. You need to fix the design. – dxiv Nov 29 '15 at 06:38
  • @dxiv I'm a stubborn programmer and that has lead me to success in my career. When someone says its not possible, there's always a way. I'm going to work on getting around this and update my question with an answer when I do... I just have to get around this, and no offense but, I don't really care about what MSDN says. – Robert Frost Nov 29 '15 at 06:48
  • @RoMa, it is in fact safe (and often necessary) to call CreateThread in DllMain if you know what you're doing. The only potential catch is that the thread won't actually start running until DllMain has exited, so you'll deadlock if you do something silly like waiting for a message from it. (But LoadLibrary is another matter.) – Harry Johnston Nov 29 '15 at 09:00
  • @Robert There's a difference between stubborn and stupid. What you are describing, "I don't really care about what MSDN says" is stupidity rather than stubbornness. Some things actually are impossible. – David Heffernan Nov 29 '15 at 10:34
0

The MCVE as posted has three problems:

  • The first is a simple mistake, you're calling IncrementReference(0) instead of IncrementReference(hModule).

  • The second is that there is no entry point for rundll32 to use; the entry point argument is mandatory, or rundll32 won't work (I don't think it even loads the DLL).

  • The third is the #pragma as pointed out by Hans.

After fixing the IncrementReference() call, removing the #pragma and adding an entry point:

extern "C" __declspec(dllexport) void __stdcall EntryPoint(HWND, HINSTANCE, LPSTR, INT)
{
      MessageBoxA( NULL , __FUNCTION__ , "Which Thread?" , 0 );
}

You can then run the DLL like this:

rundll32 testdll.dll,_EntryPoint@16

This works on my machine; EntryPoint, FirstThread and SecondThread all generate message boxes. Make sure you do not dismiss the message box from EntryPoint prematurely, as that will cause the application to exit, taking the other threads with it.

The call to LoadLibrary is still improper, however it does not appear to have any side-effects in this scenario (probably because the library in question is guaranteed to already be loaded).


(Previous) Answer:

The MCVE can be fixed by simply moving the call to IncrementReference from DllMain to FirstThread. That is the only safe and correct way to resolve the problem.

Addendum: as Hans pointed out, you'll also need to remove the /Entry pragma.


(Redundant?) Commentary:

If the application that is loading the DLL is misbehaving to the extent where the DLL is being unloaded before FirstThread can run, and assuming for the sake of argument that you can't fix it, the only realistic option is to work around the problem - for example, DllMain could suspend all the other threads in the process so that they cannot unload the DLL, and resume them from FirstThread after the call to IncrementReference.

Or you could try hooking FreeLibrary, or reverse engineering the loader and messing with the reference count directly, or removing the hooks the application has placed, or loading a separate copy of the DLL by hand inside DllMain (with your own DLL loader rather than the one Windows provides) or starting a separate process and working from there or, oh, no doubt there's any number of other possibilities, but at that point I'm afraid the question really is too broad for Stack Overflow, particularly since you can't give us the real details of what the application is doing.

Harry Johnston
  • 35,639
  • 6
  • 68
  • 158
  • The problem is, when I move it from DllMain, I get an error in my DllMain. I run the program through Rundll32, and when I do, it gives me a `Entry not found: EntryPoint` error. Even though I have a `extern "C" __declspec(dllexport) void __stdcall EntryPoint(HWND, HINSTANCE, LPSTR, INT)` in the code. – Robert Frost Nov 29 '15 at 15:52
  • See [this question](http://stackoverflow.com/q/32506036/886887). You need to either use a DEF file to remove decoration from the exported name, or tell `rundll32` that the function to call is `_EntryPoint@16`. However, if you're launching the DLL using `rundll32` then there really *isn't* any need for IncrementReference() since `rundll32` isn't going to unload your DLL until EntryPoint() returns. (It would also be more sensible to remove DllMain altogether and do everything from EntryPoint.) – Harry Johnston Nov 29 '15 at 19:53