while loop before CloseHandle()

[dellthinker]

I've been wondering if my program ever executes the following ...

HANDLE thread1=0, thread2=0, thread3=0, Array_Of_Thread_Handles[3];

thread1=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)function1,0,0,0);
thread2=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)function2,0,0,0);
thread3=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)function3,0,0,0);

while(1){
 // .. Looping some other important code 
 // .. Looping again 
}


Array_Of_Thread_Handles[0]=thread1;
Array_Of_Thread_Handles[1]=thread2;
Array_Of_Thread_Handles[2]=thread3;
WaitForMultipleObjects(3,Array_Of_Thread_Handles,true,INFINITE);

CloseHandle(thread1);
CloseHandle(thread2);
CloseHandle(thread3);

Now my first thought was that anything below the while loop would never be executed. The reason why I put it there that way is because if I put it below either WaitForMultipleObjects() or CloseHandle() the functions within the loop never executed. So I put them before those and the program executes as planned. My question is this, if there is no way out of the infinite loop then does the handles stay open? Is there no way to close them _and_ keep my infinite loop going? Thanx.

[Uglybb]

If you really do want an infinite loop (which I doubt .. we get to that in a minute) you can use the (LPVOID lpParam) within CreateThread to pass in the handle in to the thread picking it up in the thread function with a simple typecast as shown below.

DWORD WINAPI TestProcessThread (LPVOID lpParam ){
    HANDLE* HandlePtr;
    HANDLE ThreadHandle;

    // This could all be simplified to one line but doing like this for understanding
   // What you need to understand is the handle value must be quickly retrieved and held locally
   // because the passing program may trash the memory later  

   HandlePtr = (HANDLE*) lpParam;   	// Typecast to so we get the pointer passed thru in HandlePtr
   ThreadHandle = *HandlePtr;          // ThreadHandle is therefore the value at that pointer

    // Thread code goes here

   CloseHandle(ThreadHandle);          // Close the thread handle
   return (0);
};

// As per your code but I pre-assign thread1 handle to INVALID_HANDLE_VALUE rather than 0

HANDLE thread1 = INVALID_HANDLE_VALUE;

thread1 = CreateThread(NULL, 0, &TestProcessThread, &thread1, 0, 0);

It is important to realize you must take care not to immediately trash thread1 if it was within a function that dropped out for example you would be better off allocating some memory for the handle and passing that thru the thread would close the handle and release the memory

That would look like this

// A memory block is passed in containing thread handle the thread closes handle and releases memory
DWORD WINAPI TestProcessThread (LPVOID lpParam ){
    HANDLE* HandlePtr;

    HandlePtr = (HANDLE*) lpParam;   	// Typecast to so we get the pointer passed thru in HandlePtr

    // Thread code goes here

    CloseHandle(*HandlePtr);			// Close the thread handle
    free(HandlePtr);
   return (0);
}; 

// As per your code but I pre-assign thread1 handle to INVALID_HANDLE_VALUE rather than 0

HANDLE* thread1;

thread1 = (HANDLE*) malloc(sizeof(HANDLE));
*thread1 = INVALID_HANDLE_VALUE;
*thread1 = CreateThread(NULL, 0, &TestProcessThread, thread1, 0, 0);

In both of the above schemes the thread has taken responsibilty to close the thread handle.


Now from your code you shift the array handle creation above the while() loop then you can loop until all the threads complete and not have an infinite loop.

[Igor Vartanov]

My question is this, if there is no way out of the infinite loop then does the handles stay open? Is there no way to close them _and_ keep my infinite loop going?

Of course it is. You have to put your infinite loop to some other worker thread. Worker thread is the place where an infinite loop really belongs.

[Igor Vartanov]

BTW, I cannot see the reason of your concern about closing the handles a.s.a.p. You can do that after finishing your infinite loop alright. In other words, I cannot see any problem with your code.

A hint: closing thread handle does not affect the thread execution in any way.

[Uglybb]

BTW, I cannot see the reason of your concern about closing the handles a.s.a.p. You can do that after finishing your infinite loop alright. In other words, I cannot see any problem with your code.

A hint: closing thread handle does not affect the thread execution in any way.

Hint: You have not closed the handle but windows still has the resource allocated in other words you are bleeding resources each time your launch the thread if you don't close it.

from: http://msdn.microsoft.com/en-us/libr...(v=vs.85).aspx

Closing a thread handle does not terminate the associated thread or remove the thread object. Closing a process handle does not terminate the associated process or remove the process object. To remove a thread object, you must terminate the thread, then close all handles to the thread.

See the problem =====> Closing a process handle does not terminate the associated process or remove the process object ... process handle being the running now infinite loop .. so terminating it by forcably closing the application does not release the handle.

Someone needs to take responsibility to close the handle to not do so is just poor programming.

If you are going to infinite loop the program the thread needs to close the handle .. period.

[Codeplug]

>> so terminating it by forcibly closing the application does not release the handle.
When an application is terminated, it's resources are reclaimed - including open thread handles within the terminated process. The only remains of a terminated process are open handles to that process (which will then be in a signaled state).

There's nothing technically wrong with the code in post #1 (other than the infinite loop perhaps). And there's nothing wrong with closing a handle you know you'll never use. Just call CloseHandle immediately after CreateThread (no need to pass it to thread).

gg

[Arjay]

Let's look at this another way.

Handles exist so the programmer can track system objects. Once you've created the object (whether it's a process, thread, etc.), you can close the returned handle immediately if you don't need it.

If you need to use the handle further (like waiting on the handle in a WaitFor api), then don't close it until you are done using the handle (but be sure to call CloseHandle when you are finished).

Okay that being said, allow me to suggest a couple of tips for multithreaded programming.

1) Structure your code with classes (rather than in c-style).
a) Let the constructors help you with initialization of thread objects and the destructors to help with cleanup (CloseHandle, etc).
b) Let a class instance manage a thread (or manage using a thread pool). What I mean is all the thread functionality is encapsulated within the class. External callers of the class only should need to call methods that control the class (like, Start, Stop, etc) without worrying about threading details.
2) Keep the synchronization of shared data hidden within the class. It's so much easier to track down synchronization issues if the sync occurs in one spot (i.e. inside the class).
3) Let RAII help you with syncronization. Use wrapper classes around critical section, mutex, and semaphore objects. Use an autolock class obtains the lock and autoreleases the lock on the previous wrapper classes.
4) Don't use global variables. I see in previous code of an array of thread handles. Put this array inside a class (and use an stl vector) and let the class destructor clean it up when appropriate.
5) Use _beginthreadex instead of CreateThread when creating threads. Search bing or google for both key words to see why.

[Uglybb]

>> so terminating it by forcibly closing the application does not release the handle.
When an application is terminated, it's resources are reclaimed - including open thread handles within the terminated process. The only remains of a terminated process are open handles to that process (which will then be in a signaled state).

There's nothing technically wrong with the code in post #1 (other than the infinite loop perhaps). And there's nothing wrong with closing a handle you know you'll never use. Just call CloseHandle immediately after CreateThread (no need to pass it to thread).

gg

I don't believe that cleaning everything else up is techinically always true .. I can't say explicitly true because I don't know the undercode of Windows especially changes in Windows 7.

Windows does it best to cleanup resources but it is not a guarantee from the MSDN documentation

TerminateThread is a dangerous function that should only be used in the most extreme cases. You should call TerminateThread only if you know exactly what the target thread is doing, and you control all of the code that the target thread could possibly be running at the time of the termination.

For example, TerminateThread can result in the following problems:
If the target thread owns a critical section, the critical section will not be released.
If the target thread is allocating memory from the heap, the heap lock will not be released.
If the target thread is executing certain kernel32 calls when it is terminated, the kernel32 state for the thread's process could be inconsistent.
If the target thread is manipulating the global state of a shared DLL, the state of the DLL could be destroyed, affecting other users of the DLL.

A thread cannot protect itself against TerminateThread, other than by controlling access to its handles. The thread handle returned by the CreateThread and CreateProcess functions has THREAD_TERMINATE access, so any caller holding one of these handles can terminate your thread.

In a very simple or trivial application I agree you could go that route but it is certainly not what I would suggest for commercial products.

I agree with everything Arjay says and add the small bits of code and thought to sort out a proper close sequence far outways possible problems from bad coding

[Codeplug]

If not all resources are reclaimed from a terminated process, then that is a bug in the OS (not likely), or a bug in a 3rd party driver (more likely than OS bug perhaps).

This is true for a forcefully terminated process as well. But in this case I would be more worried about inconsistent data that the process itself maintains - like registry data or file data that is now corrupted/incomplete due to forceful termination.

The possible issues that calling TerminateThread/TerminateProcess may cause can not be mitigated by changing the code in post #1. Those issues exist no matter how the code written.

So normal termination is preferred, but there's little one can do, within the terminating process, to prevent issues that occur due to forceful termination.

gg

[TheGreatCthulhu]

Windows does it best to cleanup resources but it is not a guarantee from the MSDN documentation

[... docs on TerminateThread ...]

In a very simple or trivial application I agree you could go that route but it is certainly not what I would suggest for commercial products.

I agree with everything Arjay says and add the small bits of code and thought to sort out a proper close sequence far outways possible problems from bad coding

So normal termination is preferred, but there's little one can do, within the terminating process, to prevent issues that occur due to forceful termination.

Right - nobody was suggesting forceful termination: a thread will normally terminate when the thread function returns.

So, the goal is to design the app so that the thread function returns at some point - if this function contains an "infinite" loop, than it's just a matter of modifying the value of a bool variable that controls the loop (when the work is done or not required anymore - set to false).

As for the handles - these are basically internal system-specific references to kernel objects. Kernel objects are just some data structures used by the OS to keep track of things like threads or processes, and to store some information about them that your application may require. The thread or a process can be long gone, but if some other application still holds the handle, the kernel object will still be there - and this is perfectly normal - an app might process the associated information of a finished thread for one reason or the other. So, a thread kernel object and a thread are associated, but are not one and the same thing.
Once all the handles are closed, the system will do cleanup, by design. (Since the information is no longer required.)

As for the possibility of bugs in the OS or a 3rd-party driver: these are very specific cases; again: in normal circumstances, the system will preform cleanup.
(But, I'm not saying that the app shouldn't do it's own cleanup, too.)

P.S: BTW: The design advice provided by Arjay is very good.

[Arjay]

Sometimes developers want to justify why they don't clean things up properly. Like not calling ::CloseHandle( ) when they don't need a handle or are finished using one.

Sure eventually as mentioned the system will free up the underlying kernel resources (at the very latest, when all processes using the object are no longer running, the system will clean up).

The problem is twofold: 1) the not cleaning up handle strategy isn't going to work for long running processes like services; 2) You are reinforcing sloppy programming practices.

It all boils down to doing what is right. If you train yourself to close handles (and in general free up resources) as soon as you are finished with them, you won't find yourself in a situation where you have product running at a client location that runs for a while and then gets bogged down and crashes.

It's a pain in the butt to go back over someone's code and fix up memory and handle leaks. It's so much easier to follow good programming practices and not put the leaks in there to begin with.

Especially if you use OOP and classes. Initialize handles to null in the ctor, set them where you need them and in the dtor, close the handles.

When you declare a class handle variable, train yourself to always intialize handles in the ctor and immediately add a closehandle (or delete) call in the dtor. If you do this, you will greatly reduce the chance for leaking handles and memory.

[TheGreatCthulhu]

I was commenting on Uglybb's post; but I should have probably put more emphasis on this line:

in normal circumstances, the system will preform cleanup.
(But, I'm not saying that the app shouldn't do it's own cleanup, too.)

So, yeah - what Arjay said.

[Abbyabbie]

I've been wondering if my program ever executes the following ...

HANDLE thread1=0, thread2=0, thread3=0, Array_Of_Thread_Handles[3];

thread1=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)function1,0,0,0);
thread2=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)function2,0,0,0);
thread3=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)function3,0,0,0);

while(1){
 // .. Looping some other important code 
 // .. Looping again 
}


Array_Of_Thread_Handles[0]=thread1;
Array_Of_Thread_Handles[1]=thread2;
Array_Of_Thread_Handles[2]=thread3;
WaitForMultipleObjects(3,Array_Of_Thread_Handles,true,INFINITE);

CloseHandle(thread1);
CloseHandle(thread2);
CloseHandle(thread3);

Now my first thought was that anything below the while loop would never be executed. The reason why I put it there that way is because if I put it below either WaitForMultipleObjects() or CloseHandle() the functions within the loop never executed. So I put them before those and the program executes as planned. My question is this, if there is no way out of the infinite loop then does the handles stay open? Is there no way to close them _and_ keep my infinite loop going? Thanx.

so well