How to close a thread when uninjecting a dll?

[trafficlights7]

Hi all,
I've got a dll that is injected into remote processes and it uses CreateThread which works fine.

Problem is when i uninject the dll, the remote process crashes.
I also tried using NewThread from madKernel which created the thread ok, but again it crashed the remote process when uninjecting the dll . . .

What am i doing wrong in the code snippet below?
Is there a better method to do this?


function WorkerThread(argParams : Pointer) : LongInt; stdcall;
begin

while (Running) do
begin
Sleep(1000);
//do some stuff
end;

end;


procedure EntryPoint(Reason: dword);
begin

case Reason of
DLL_PROCESS_ATTACH:
begin
// setup API hooks etc
Running := True;
lThreadHandle := CreateThread(nil,0,@WorkerThread, nil,0, lThreadID);
end;

DLL_PROCESS_DETACH:
begin
//release API hooks
Running := false;
CloseHandle(lThreadHandle);
end;

[dcsoft]

What am i doing wrong in the code snippet below?
Is there a better method to do this?


function WorkerThread(argParams : Pointer) : LongInt; stdcall;
begin

while (Running) do
begin
Sleep(1000);
//do some stuff
end;

end;


procedure EntryPoint(Reason: dword);
begin

case Reason of
DLL_PROCESS_ATTACH:
begin
// setup API hooks etc
Running := True;
lThreadHandle := CreateThread(nil,0,@WorkerThread, nil,0, lThreadID);
end;

DLL_PROCESS_DETACH:
begin
//release API hooks
Running := false;
CloseHandle(lThreadHandle);
end;

Well, when you set Running to false, it could take up to 1 second for the thread to poll this and exit. Meanwhile, you've continued on and closed the thread handle and exited DllMain(), which promptly uninjects the dll before your thread has a chance to really exit.

If you have nothing better, after setting Running = false, you could start a loop calling GetExitCodeProcess() until it returns something besides STILL_ACTIVE. This guarantees that the thread quits before you exit DllMain and the DLL is uninjected.

-- David

[madshi]

David is half right. Basically he's right. There's just one little problem: While DllMain is running, no other thread can run. So waiting for that other thread to finish will mean you wait forever. If at DllMain that secondary thread is still running, you have no other choice than to terminate it by calling TerminateThread. However, that's no clean approach.

Do you really need that thread? Can't you get along without it? A hook dll which is injected into a multitude of processes should try to have as little impact on the host processes as possible. Creating threads is possible, but if you can find a way to get along without it, better do that.

If you urgently need an additional thread, you should try to close it down, before you do the uninjecting. You could do that e.g. by creating a global event ("CreateGlobalEvent") in your application and let the thread check it ("WaitForSingleObject(event, 0)") instead of checking that "Running" flag. Then you could signal that global event in your application and wait a second or so before starting the uninjection.