Click here to Skip to main content
15,886,074 members
Please Sign up or sign in to vote.
4.00/5 (1 vote)
See more:
INTRODUCTION AND RELEVANT INFORMATION:

I have a main window with a button.

On button click a dialog box is shown.

Dialog box has a button that spawns a thread.

Thread function communicates with the dialog box via 2 custom messages.

Right now I am using a boolean variable the way it was used in the thread example from book Programming Windows, 5th edition-Charles Petzold.

I wish to improve the code for thread synchronization, namely to substitute the usage of boolean and global variables for something more efficient.

The important information is also the way threads are aborted when user presses Alt+F4 or X button on either dialog box or main window so here is the explanation of the algorithm I have deployed:

If the user tries to close the main window while thread is still running, I set the global boolean variable and leave WM_CLOSE handler without destroying the window. I also send WM_CLOSE to the dialog box.

In the dialog procedure, in WM_CLOSE handler, I check if thread is still running and if it does I send abort signal and return without destroying the dialog box.

Now both main window and dialog box are alive and wait for thread to finish.

Once thread sends custom message, informing the dialog box it has finished, I check the global boolean variable and if it is set I post the WM_CLOSE to the main window and I post WM_CLOSE to the dialog box if the user requested to close it.

Close request for dialog box is stored in a boolean variable that gets set when user tries to close dialog box.

As far as thread function is concerned, I check the value of bContinue periodically and abort thread properly when it is set to false.

Here are the relevant code snippets:
C++
//GLOBAL VARIABLES:

struct Data
{ 
    bool bContinue; // continue thread execution?
    // other data
};

//global variable that indicates the main window's close request
static bool g_closeApp; 

The main window's WM_CLOSE handler:
C++
case WM_CLOSE:
    closeApp = true;
    /******* close dialogs ****/
    if( IsWindow( hDlgTSO ) )
    {
        SendMessage( hDlgTSO, WM_CLOSE, 0, 0 );
        return (LRESULT)0; //<-------return and wait for thread to finish
    }
    // perform usual cleanup and destroy window
    return (LRESULT)0;

The relevant dialog box snippets:
C++
INT_PTR CALLBACK DlgProc(HWND hwnd, UINT Message, WPARAM wParam, LPARAM lParam)
{
    static HANDLE threadHandle;
    static Data data; //data sent to the thread function
    static bool closeDlg; // close dialog requested?

    switch(Message)
    {
    case WM_INITDIALOG:
        {
            closeDlg = false;
            threadHandle = NULL;
            // other stuff...
        }
        return TRUE;

    case WM_COMMAND:
        {
            switch(LOWORD(wParam))
            {
            case IDOK:
                {
                     // create thred
                     DWORD threadID;
                     // initialize thread data
                     data.bContinue = true;

                     threadHandle = CreateThread( NULL, 0,
                                       (LPTHREAD_START_ROUTINE)ThreadFunction,
                                       (void*)&data, 0, &threadID );

                     if( !threadHandle )
                     {
                         MessageBox( hwnd, L"Error!", L"Error", MB_ICONERROR );
                         DestroyWindow(hwnd); // destroy modeless dialog box
                     }

                 }
                 break;
        }
        break;

    case WM_CLOSE:
        closeDlg = true; // user wants to close dialog
        if( threadHandle ) //if thread is running signal abort and wait
        {
            ShowWindow( hwnd, SW_HIDE );
            data.bContinue = false; 
        }
        else
        {
            if( closeApp ) // close main window?
                PostMessage( GetParent( hwnd ), WM_CLOSE, 0, 0 );

            threadHandle = NULL;
            DestroyWindow(hwnd);
            hDlgTSO = NULL; // needed-this is modeless dialog!
        }
        return TRUE;

    case WM_THREAD_OK: //custom message, indicates successful thread execution 
        if( threadHandle ) // wait for thread to fully exit
        {       
            WaitForSingleObject( threadHandle, INFINITE );
            CloseHandle( threadHandle );
            threadHandle = NULL;
        }

        // if in the meantime user pressed X or Alt+F4 post WM_CLOSE
        if( closeDlg )
            PostMessage( hwnd, WM_CLOSE, 0, 0 );
        return TRUE;

    default:
        return FALSE;
    }
    return TRUE;
}

The thread function:
C++
DWORD WINAPI ThreadFunction( LPVOID pvoid )
{
    volatile Data* data = ( Data* )pvoid;
    //do something with data..
    if( ! data.bContinue ) // abort signal is set
    //do cleanup and return
}

MY QUESTION:

Is there a better implementation which would enable me to remove boolean variables, especially the global one?

I do not seek suggestion for synchronization mechanism as I believe that current one is OK, but if anyone has better suggestion I will consider it.

Thank you.

Best regards.
Posted
Comments
pasztorpisti 26-Jan-14 16:58pm    
If you want to eliminate global variables: A global variable is nothing more then a "few bytes" of memory on a fixed offset inside the memory address space of your process. The address that holds your data could be anywhere else, even on a dynamically allocated memory area but in that case you have to pass the pointer as a parameter to both threads. On the other hand you have to make sure that you handle correctly the ownership of the dynamically allocated memory area. If you know that that one of the threads is destructed earlier than the other then you can simply put the deletion of your dynamically allocated memory into the thread that lives longer. Or you can give the ownership of the memory area to a third object that lives even longer than your two thread objects (for example an app object). This way you can guarantee that when the deletion/freeing of the memory area occurs only one of the "owner" objects/threads is alive so it doesn't happen that you free a block of memory that is still used by another thread. The dynamically allocated memory area can be anything ("p = new bool" :-) or a new struct).
Sergey Alexandrovich Kryukov 26-Jan-14 18:22pm    
This is very, very non-trivial problem. I wrote on it in the past and wrote in reply to this question, please see.
—SA
AlwaysLearningNewStuff 26-Jan-14 18:43pm    
"On paper" I understand what you are saying, but I fear that I will miss something out, since thread synchronization with volatile keyword and bool variable seems a little suspicious to me. That is why I have asked the question, so experts can recommend the best way this can be done. By the way, the "thread communication" is actually implementation of your solution to my earlier answer, and I really like it. Perhaps you could just cast a quick glance at my code/concept and suggest me a more efficient way of implementing synchronization? I am aware I might be asking too much, and if you decline I will understand. Still, since I value your opinion highly, I am very interested in your suggestion. Thank you for your comment. Best regards.

1 solution

Well, of course this implementation is inefficient as all those techniques based on polling. First, one side note: your use of struct Data is totally pointless, because it is equivalent to using static bContinue along. Also, using any global variables is generally bad, you could try to use, say, thread-local storage. However, all that would not help to solve the bigger problem.

All that stuff is reduced to really serious problem: can a thread be terminated in a way totally asynchronously to the execution of the thread code. Actually, the answer would be "yes", because you could always use non-graceful TerminateThread (please, never do it!). So, more essential question would be: can a thread be terminated in a way totally asynchronously to the execution of the thread code in safely? And this problem remains a problem.

To start with, I'm going to make a very strong, controversial and unusual statement: the developers of most operating systems did not understand the requirements imposed by the application. The usually recommended practice is: use cooperative approach. It means: the approach you just used. The OS developers did not realize that there are applications where such approach it totally unsuitable; and I can give you a number of examples. Imagine you are launching a complex calculation task which does not have distinct periodic structure. Try to use some numeric methods of solution of the system of differential equations and you will understand what I mean. The time spans needed for execution of certain cycles are simply unpredictable. Besides, you cannot distract the developer from important numerical-method problem to stupid thread cooperation, which is totally unrelated. There are many other examples where you really need to abort a thread. How to do it safely? The question is really a big problem, and this is so just because OS developers did not realize the importance of the problem well enough. If something is really needed for solution of the application problem, application developer will use non-graceful approach, so banning asynchronous termination can make things only worse.

I found a real solutions years ago and unfortunately lost the reference to original work. But I can describe it in detail. Please see my past answer: Close correcly the thread inside a dll[^].

On the concept of preemption, please read: http://en.wikipedia.org/wiki/Preemption_%28computing%29[^].

One final note. The solution described above is save and efficient, but it was never accepted in Windows API. Nevertheless, it made it way into much wiser API, which is .NET. This is how .NET FCL Thread.Abort works.

This issue is very complex and non-trivial, so your follow-up questions will be very welcome.

—SA
 
Share this answer
 
v3
Comments
AlwaysLearningNewStuff 26-Jan-14 18:38pm    
It is 0:00 PM now, and I do not wish to approach this topic without being fully rested. Your answer is very helpful and informative as always. Looking forward reading the content of the posted links. Although I realize my question may be broad, I just strive to write as efficient code as possible-thank you for helping me learn how to do that. Until I read content from your suggested links I great you. Best regards Mr.Kryukov!
AlwaysLearningNewStuff 26-Jan-14 19:02pm    
I was able to give a quick look for the answers/articles you linked to and I like what I saw ( +5 from me )-I just fear that I will screw up somewhere being inexperienced. Still, I will give this more attention starting from tomorrow. Very instructive and informative answers-thank you! Best regards!
Sergey Alexandrovich Kryukov 26-Jan-14 19:18pm    
You are welcome, and thank you.
This is not so hard to implement, but yes, it needs attention, because screw-ups can dramatically break the functionality...
—SA
pasztorpisti 27-Jan-14 11:16am    
You are quite right about the separation between the actual problem solving and the handling of thread/task cancellation.
I remember we were talking about this technique at least twice in the past. The concept itself is brilliant but the usage of this technique in a language that manages resources "manually" like C/C++ throwing an unexpected exception results in tons of resource leaks. I think it isn't surprising that the technique is implemented only in languages with garbage collection. In C/C++ you have to deal not only with the problem of storing everything on the stack with RAII (you can not force this, someone will sooner or later will place a raw pointer there...) but you have to put be able to enable/disable the exception - I remember we were talking about this, and these exceptions should be thrown by api functions that check the "Abort-Request" flag (similarly to the posix thread cancellation). The implementation is not too difficult (very hard for someone unexperienced in threading) but it would be a bit dirty (checking abort flags here-and there and making sure that you call at least 1-2 functions in your thread with abort check) and yet unsafe. I wouldnt like to use it in a c/c++ project or with any other language that has no garbage collection. In a C/C++ application I would go with message passing whenever possible and signalling with a flag if I want to "send a message" to a "long running job". C/C++ was designed for bare metal plumbing and unfortunately implementing this technique properly isn't possible, in my opinion it solves a problem but brings a lot of other problems.
Sergey Alexandrovich Kryukov 27-Jan-14 11:36am    
I understand your considerations, but I still maintain that the technique is quite practical if used correctly. You need to solve some non-trivial problem with this technique, to be really able to estimate it. Basically, the possibility to do post-mortal processing in abort exception solves all the problems.

However, things of course are not so simple. One reader once gave me the argument that the aborting during complex construction is absolutely impossible to perform correctly, because I would up with plenty of half-constructed object. But this problem does have resolution: you need to be able to suspend and resume the "abortability" of the thread. All methods of doing so should be based on thread synchronization. Say, it can be a "non-abortable mutex", which is easy to implement. Of course, the thread code should guarantee the absence of deadlocks based on this mutex (in particular).

The real problem of this technique is the lack of platform-independent code: the core code for GetThreadContext and SetThreadContext is official API, but the context depends on CPU architecture.

—SA

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