Click here to Skip to main content
15,881,204 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Hello my fellow colleagues from CodeProject!

I will try to be brief, so I will cut to the point:

I work on Windows XP, in C++, using pure Win32 to create a dialog box.

That dialog box has some edit controls, and OK button, which activates a thread when pressed.

Thread then gathers text from edit controls and writes them into MS Word document, using OLE Automation.

Everything works fine,when I press OK button, and wait for thread to show filled Word document.

However, when I push the OK button and then close the dialog, while thread is in the middle of the work, a blank Word document pops up.

To further illustrate my problem here are some code snippets:

This is snippet for thread function:

C++
DWORD WINAPI TabelaSvihObjekata( LPVOID hWnd ) // hWnd is handle of the Dialog box
{
        // obtain dialogs window handle

	HWND hwnd = (HWND)hWnd;

	// Initialize COM for this thread...

	CoInitialize(NULL);
	
	// Get CLSID for our server...

	CLSID clsid;
			
	HRESULT hr = CLSIDFromProgID(L"Word.Application", &clsid);

        // do other Automation stuff and clean afterwards
}


In dialog box, this is the snippet for button handler:

C++
case IDOK:
{
	// create thread

	DWORD threadID;

	HANDLE threadHandle = CreateThread( NULL , 0 , 
                                (LPTHREAD_START_ROUTINE)TabelaSvihObjekata , 
                                (void*)hwnd , 0 , 
				&threadID );

	if( !threadHandle )
	{
		MessageBox( hwnd, L"Error", L"Error", MB_ICONERROR );

		EndDialog( hwnd, IDCANCEL );
	}

	CloseHandle( threadHandle );

}
break;


And this is the problematic handler:

C++
case IDCANCEL:

        EndDialog(hwnd, IDCANCEL);

	break;


My question is:

What should I add or change, so when I close the dialog box, Word application closes along with threads destruction ?

I have looked on MSDN for a clue, and have found only ExitThread as a solution, but I don't know how to use it properly since I am inexperienced with threads.

If there is anything else that I can do to help, ask and I will gladly do it.

Thanks to everybody who tries to help.

EDIT #1:
-----------------

According to suggestion made by member pasztorpisti, the improvements that I have came up with will be shown bellow.

Before that, I must point out that I didn't test this code, since I would like it to be verified first by experienced colleagues.

Here are the code snippets, discussed above:

Thread function changes:

-Passed LONG value instead of HWND to terminate thread properly.
-Query is performed inside thread function, so the GUI can be left "untouched".

Sample snippet:
C++
DWORD WINAPI TabelaSvihObjekata( LPVOID StopThreadExecution ) 
{
        // OBTAIN DIALOGS EXIT VARIABLE

	LONG  exit = ( LONG ) StopThreadExecution;
 
	DWORD result = 0; // the thread functions return result  

	while( InterlockedExchange( &exit , 0 ) != 0 )
       {
		try
		{

       			// Initialize COM for this thread...

	    		CoInitialize(NULL);
	
        		/***** execute query here *****/

        		/**** then open Word 
                        and fill it in with the data from query ****/

			// Get CLSID for our server...

			CLSID clsid;
			
			HRESULT hr = CLSIDFromProgID(L"Word.Application", &clsid);
 
        		// do other Automation stuff 
		
                        break; // Word is filled, exit loop and do cleanup
		}
		catch( _comm_error & )
		{
			result = 1; /** error occurred, 
                                    change return value to indicate error **/
   			
			break; // exit loop
		}

	}
		
	// do cleanup 
        
	return result;
}


Dialog box changes:

-Added LONG variable which will be used for stopping the thread.
-Changed the declaration of thread handle, so WaitForSingleObject can be used properly.
-Changed parameter for CreateThread in IDOK handler.
-Added statement that stops thread, and WaitForSingleObject to IDCANCEL handler.

Sample snippet:
C++
LONG  StopThreadExecution = 0;
HANDLE threadHandle = NULL;

case IDOK:
{
	// create thread

	DWORD threadID;
 
	threadHandle = CreateThread( NULL , 0 , 
                                (LPTHREAD_START_ROUTINE)TabelaSvihObjekata , 
                                (void*) StopThreadExecution, 0 , 
				&threadID );
 
	if( !threadHandle )
	{
		MessageBox( hwnd, L"Error", L"Error", MB_ICONERROR );
 
		EndDialog( hwnd, IDCANCEL );
	}
 
	// I have tried here with WaitForSingleObject,
	// but it blocked the dialog box, until thread returned.
	// Since the user should be able to do other stuff,
	// like minimizing or selecting from combo box, I have deleted that line


	CloseHandle( threadHandle );
 
}

break;

case IDCANCEL:
 
	InterlockedExchange( &StopThreadExecution, 1 );

        if( threadHandle != NULL )
	        WaitForSingleObject( threadHandle, INFINITE ); 

        EndDialog( hwnd, IDCANCEL );

	break;
Posted
Updated 11-Aug-13 13:34pm
v6
Comments
Maciej Los 10-Aug-13 15:56pm    
Can't you just hide dialog box on OK button click event? This will avoid close button click... ;)
MyOldAccount 10-Aug-13 17:27pm    
Unfortunately no I can not.

Your solution has several serious multithreading related problems. You are executing a long running task. A long running task is often but not necessarily cancelable.

Here is a few rules that you should never break in case of multithreading and in case of a gui program:
- The ownership of worker thread object (thread lifetime): Every worker thread must be waited/joined before the program terminates. You must store the thread handle and you have to wait for it with WaitForSingleObject later and then you can close the handle. Of course you want to do that by minimizing the wait time on the gui thread.
- In case of well designed multithreading the thread always terminates by returning from the thread function and not by calling ExitThread or another evil function. This is very important in case of C++. TerminateThread is even more evil.
- A worker thread is not allowed to call any gui related functions, it shouldn't manipulate your gui elements and editboxes/other controls. Gui systems are usually single-threaded and the manipulation of gui elements can be done only by the thread that created the control on windows. Maybe the only function calls you can safely use from worker threads are PostMessage() and SendMessage(). I'm really interested how do you gather info from editboxes, probably its hell buggy and its a miracle if you haven't crasted because of incorrect usage of your gui from multiple threads. Post some more code and we will spot the problems in it.

Find out how should your program behave from the users perspective. I would tie together the lifetime of the dialog and the worker thread and I wouldn't allow closing the dialog before the worker thread termination. Depending on the kind of blocking function calls performed by the worker thread the user may have to wait for a short period of time when the cancel button is pressed, in worst case you have to put out a "terimnating background task" label with an animating something to provide better user experience.

Post more code. It is very important to see what do you do on the thread.

EDIT:
General rule: If a piece of data or dataset may be modifed from one thread, then you are not allowed to access it by any of the other threads without synchronization, without using the data by cooperation between the threads with a lock. Every thread is allowed to access its own data, any kind of shared data that is read only for the duration of the task a thread performs, and all kind of data that may be modified by other threads can be accessed only if all threads access that data with synchronization/cooperation using locks. It is also possible to pass the ownership of data from one thread to another thread and to ask the other thread to do something with that data. The SendMessage() and PostMessage() functions are for this purpose, you can use them to send data to the gui thread and to ask the gui thread to perform some kind of task for you. Define your won window messages (WM_APP+XXX or WM_USER+XXX) and send the progress of your background task (if known) and when the task is done you can send a final message with some additional info (success/failure). By sending a progressupdate message you can safely handle that on the gui thread and you can update for example a progressbar. If the data to send is large (more than 1 or two integers/pointers) then allocate a piece of memory or a result object (with new) and pass the pointer with the message and release the memory/object on the other thread after processing the data. From the worker thread you should use only PostMessage or SendMessage but rather PostMessage because why would you need for waiting for the other (gui) thread to process your message??? So we solved the problem of sending messages from the worker to the gui thread at any point of the life of the worker. Now how to communicate from the gui thread to the worker for example to send cancel or pause??? Unfortunately this can be done correctly only by sharing a piece of data between the worker and the gui thread and from the worker thread must check the value of this data regularly. This way you can set the cancel flag in the shared data and next time the worker checks the value it knows that it has to exit. From the gui thread you set the cancel flag and you wait for the thread to terminate patiently. No ExitThread. No TerminateThread. Pause must be implemented similarly but you probably dont need this. The data sharing can be implemented in several ways, probably the simplest and easiest in case of a simple signal like cancel is something like this:
C++
// Provided AS IS without warranty, untested, use it at your own risk!!!
class SharedSignal
{
public:
    SharedSignal()
        : m_Val(0)
    {}
    bool ReadAndResetSignal()
    {
        return 0 != InterlockedExchange(&m_Val, 0);
    }
    void Set()
    {
        InterlockedExchange(&m_Val, 1);
    }
private:
    LONG m_Val;
};

The problem for example with calling gui functions like GetDlgItemText() from the worker is that who knows what kind of data does windows uses to handle the state of that dialog control... And what happens to that data if multiple threads calling that function by accessing/modifying the state of those dialog controls at the same time... Windows gui has been designed to be single threaded and you have to respect that. You should gather the data from the controls on the gui thread before starting the worker and you should pass that input data as createparams to the thread. There is a void* input parameter for the thread proc, you can specify any kind of pointer to pass in for the thread when you call CreateThread. Allocate memory/object on the gui thread, collect all data and pass it as input parameter when you create the thread. On the thread receive and use the data and free the memory/object when no longer needed.

Summary:
- You should collect all data from gui controls on the main thread before starting the worker
- Communication between gui and worker while the worker is active: you use PostMessage() to pass progress and finish messages to the gui.
- Use a shared signaling mechanism between the worker and the gui to cancel the worker progress, I provided one possible solution. Make sure you don't poll this signal too often from your worker as it involves memory synchronization that may slow down your worker, in your case this probably isn't an issue because you are probably executing long-blocking functions.
- In your gui thread you have some kind of state information about every worker threads, this info is often just the fact that they exist/active and sometimes their progress. You modify this gui-side info only by receiving the progress/finish messages of the workers and you are allowed to politely ask them to finish by sending cancel signal. Even when the user interacts with your gui you are allowed only to check this gui-side information and sending the signals. For exmaple if the user presses the "brutal" X button of the main window you still have to handle that somehow without exiting before the worker tasks finish. In either case you have to handle the WM_CLOSE of your main window because that would terminate your program. From the WM_CLOSE you can check the gui-side state of the workers and as a dumb solution you can popup for exampel a messagebox telling that some backgroudn tasks are still active. By handling WM_CLOSE you can prevent X and alt+f4 from closing/destroying your window. If no workers are active then you can let the inerhited WM_CLOSE handler to do its job and destroy the main window and terminate your main message loop and the application.

Another WM_CLOSE handling as an alternative to the messagebox: set an internal flag ("exit_request") and show a modal dialog with an animation and label ("exiting..."). The modal dialog prevents the user from interacting with other windows and in your exiting modal dialog you should handle the WM_CLOSE in order to prevent the user from closing it with X or alt+f4. All the time you receive a finish message from a worker thread you check the state of all threads and also the exit_request flag. If there are no more running workers and the exit_request flag is set then you can resend the WM_CLOSE message to your app programmatically: http://support.microsoft.com/kb/117320[^].
 
Share this answer
 
v6
Comments
MyOldAccount 10-Aug-13 15:07pm    
I have tried posting code, but it failed, since the code is quite long.

I will try to explain what I did:

Dialog box should present the user data from a database, when created.

After user clicks on print button, filled Word document should pop up as a result.

To do this I have made a thread function that has a HWND of a dialog box as a parameter ( see the above snippet for TabelaSvihObjekata to see what I am talking about ).

Then, I use OLE Automation to open word document and populate it with the data from dialog box with following method:

Since dialog has edit controls, I retrieve their text in worker thread via call to GedDlgItemText API.

Then I write the data into word using OLE Automation.

Does this help anyway?
pasztorpisti 10-Aug-13 15:11pm    
You are not allowed to call GetDlgItemText() in a worker thread. BTW, lets start with simpler questions: why are you using multithreading here?
MyOldAccount 10-Aug-13 15:19pm    
Dialog box has combo box with the list of buildings, which information can be showed in Word document.

In order to allow user to work on something else, while Word is being populated, I've decided to use threads.
pasztorpisti 10-Aug-13 15:27pm    
I'm afraid you shouldn't complicate things with threading here. Threading is a dangerous toy. How long does it take for the task to complete and what else can the user do while this task is in progress? What does the user see on the screen? How do you imagine the use cases of the software with background tasks? Where is it possible for the user to navigate on the gui while the background task is running? What else is allowed during the background task?
MyOldAccount 10-Aug-13 15:33pm    
Well, the user can use combo box to load information about another object while Word is being filled.

Also, he/she can minimize dialog box and activate other features of the main GUI, since there are other buttons that activate some other dialog boxes as well.

As for the task duration, well, document is small, it takes around 30 to 60 seconds to fill it.
Hi Nenad,

I suggest, you should use thread synchronization mechanism such as a Event to do you job perfectly. And then use WaitForSingleObject windows API to wait for the event to be signaled. When done with your thread, I mean after completing the writing to word document, set the event as signaled. Use ShellExecute API to open the word document using MS Word. I hope this make sense

Thanks & Regards,

Nitheesh
 
Share this answer
 
Comments
MyOldAccount 10-Aug-13 14:09pm    
It makes perfect sense to me Nitheesh! Thanks for a reply, I highly appreciate it!
I use OLE Automation to fill in Word, and it works great, so I don't need Shell APi call.
However, I am interested in using Event and WaitForSingleObject, but I just don't know how to use them since I am new to threads.
I have tried with WaitForSingleObject before but it didn't turn out well, so some pointers or code snippets would be usefull.
I will try to search online for code examples, and will report back my progress.
Again, thank you so much!

P.S. You have posted your answer twice :))
MyOldAccount 11-Aug-13 19:56pm    
Nitheesh, I have a question:

Will WaitForSingleObject block my entire dialog box ?

I need that dialog box to be free for user to do other things like minimize, clicking on other controls ...
Others have posted here some very good advice, which I will not contradict. Several comments from me:
(1) your IDOK handler does not end with a "break;" statement, and as shown, will drop through to the IDCANCEL code. In the IDOK logic, you close the handle, then use it in the IDCANCEL logic. This won't work. If you mean to execute the IDCANCEL logic after IDOK, you probably want to move the handle close statement to the IDCANCEL section.
(2) you said yourself that you are a novice programmer (I believe you) but you are doing some threaded code. This is a dangerous mixture. You should become a more experienced developer before going on an adventure with threads. You have made a lot of fundamental mistakes in the code you present here.
 
Share this answer
 
Comments
MyOldAccount 11-Aug-13 17:53pm    
Thank you, for your remarks.
I know this is huge for me, but my employers have changed the terms in the middle of the project, so I have no choice but to do this.
As for break; it was a typo.
Can you help please?
I doubt a novice can "push through" this on his own.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900