C / C++ / MFC

25-Dec-08 16:26

I am writing any application using C++ and MFC. Please consider the following two class member functions:

<br />
	void ReportASuccess()<br />
	{<br />
		successCount ++;<br />
		this->Invalidate();<br />
	}<br />
<br />
	void ReportAFailure()<br />
	{<br />
		failCount ++;<br />
		this->Invalidate();<br />
	}<br />

There two functions will be called many thousands of times by several worker threads to report results. I am concerned that the increment of the counters successCount and failCount is not thread safe. That is, if the thread loses focus in the middle of the increment, there could be a problem. Do I need to worry about this, and if so, what is the way around it?

By the way, the two member functions shown above are part of class that handles the GUI.

Thanks

Bob

Luc Pattyn25-Dec-08 17:29

Luc Pattyn

25-Dec-08 17:29

Hi,

if this is a GUI part, i.e. some Control (maybe a Form), then those functions
have to be executed by the main thread aka GUI thread, hence successCount and failCount
are single-threaded variables; the GUI thread cannot preempt itself.

If your worker threads are to call these methods, you need a mechanism to defer the execution
to the GUI thread, something similar to Control.Invoke in .NET

FYI: the general solution for thread-safety involves synchronization of some kind, here
locks could be appropriate; you are aware of the _Interlocked functions, such as _InterlockedIncrement, are you?

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

Fixturized forever. Confused | :confused:

Code-o-mat25-Dec-08 22:51

25-Dec-08 22:51

For example, create a mutex using CreateMutex[^] and use WaitForSingleObject[^] to wait for access, something like this:

void ReportASuccess()
{
  if (WaitForSingleObject(handle_to_your_mutex, INFINITE) == WAIT_OBJECT_0)
  {
    successCount ++;
    this->Invalidate();
    ReleaseMutex(handle_to_your_mutex);
  }
}

void ReportAFailure()
{
  if (WaitForSingleObject(handle_to_your_mutex, INFINITE) == WAIT_OBJECT_0)
  {
    failCount ++;
    this->Invalidate();
    ReleaseMutex(handle_to_your_mutex);
  }
}

If your Invalidate() method belongs between WaitFor... and Release depends on what it does. Generally, i think, a thread will not loose control over the CPU in the middle of an increment/decrement operation, however, this might change with a multicore or multiprocessor system.

As an alternate solution, if your ReportASuccess and ReportAFailure methods are used to control the GUI thread, you could use PostMessage[^](or rather PostThreadMessage[^]) to signal events (just try not to flood your GUI thread with messages).

On a sidenote, be careful with updating the GUI from a different thread than it was created in because this might -and probably will- lead to failures and deadlocks.

> The problem with computers is that they do what you tell them to do and not what you want them to do. <

BobInNJ26-Dec-08 6:16

26-Dec-08 6:16

Thanks for the response. I am leaning towards using the PostMessage approach. I am wondering why you said PostMessage rather than SendMessage. Is one better than the other?

I am also thinking that if I wanted to use the locking approach, that I should use two locks. That is, when one thread is reporting success, the other thread can report failure. Maybe I did not give you enough information. Each thread represents a simulation of a stochastic process. Some times, due to luck, they will report success. Other times, they will report failure. What user cares about is the percentage of time they report success. Right now, it is taking about 10 milliseconds per simulation and when the program is put into use, the user will be doing about 1 million simulations per run.

With that said, if I use the locking approach, is there any reason to use only one lock?

Thanks

Bob

Code-o-mat26-Dec-08 7:57

26-Dec-08 7:57

If you use SendMessage you are actually calling the message handler directly from the same thread you called SendMessage in, while if you use PostMessage, you are placing the message in the other thread's message queue and it will handle the message when it gets to it in its own thread. So basicly, if you use SendMessage, the message handler will run in the same thread that called SendMessage, if you call PostMessage, the message handler will run in the GUI thread (or at least the thread that owns the message queue).

For the locking aproach, you can use 2 different locks, there was no special reason why i used the same mutex, and using 2 different locks is probably better for performance, i was just lazy...sorry. Smile | :)

It really mostly depends on what resources the two methods access, since you are increasing two different variables you don't need to have the same lock for both, but if they use the same Invalidate method which might access resources which should not be accessed by multiple threads at the same time you would need to use the same lock. Depends on the situation and structure.

> The problem with computers is that they do what you tell them to do and not what you want them to do. <

BobInNJ26-Dec-08 8:28

26-Dec-08 8:28

Thanks for the response. I noticed that there are three PostMessage functions. One is in the Windows API, one is a member of the class CWindow and the other is a member of the class CWnd. Since I want the message to be processed by the GUI thread, your advice on PostMessage vers SendMessage is right on target. However, I am not sure which of the three PostMessage functions to call. I am thinking that I should call CWnd::PostMessage(). Does it matter which one I call?

I would also like to say that the codeproject has quickly become one of my favorite websites due to people like Code-o-mat.

Bob

Mark Salsbery26-Dec-08 10:20

Mark Salsbery

26-Dec-08 10:20

FYI - It is NOT true that SendMessage() called from a different thread the window was
created on calls the windowproc on the different thread. If it's called from a different
thread, that thread is blocked and the WindowProc is called on the thread the window was created on.
So you can use ::SendMessage() from a different thread than the window was created on, but be
aware the blocking nature is different than the non-blocking nature of ::PostMessage().

Also, if those variables you are incrementing are integers <= the native integer size
for the processor you're running on, then the increment operation is most likely atomic
so it is thread safe. The variables should be marked as volatile, however, or better yet,
use interlocked increment operations as mentioned by Luc.

The Invalidate calls are NOT thread safe if "this" is a CWND (or derived) pointer.

If you end up locking the operations, you may want to use a critical section instead of a mutex.
You only need a mutex if other processes need synchronized access...

Mark

Mark Salsbery
Microsoft MVP - Visual C++
Java | [Coffee]

Code-o-mat26-Dec-08 10:56

26-Dec-08 10:56

Mark Salsbery wrote:
FYI - It is NOT true that SendMessage() called from a different thread the window was
created on calls the windowproc on the different thread. If it's called from a different
thread, that thread is blocked and the WindowProc is called on the thread the window was created on.
So you can use ::SendMessage() from a different thread than the window was created on, but be
aware the blocking nature is different than the non-blocking nature of ::PostMessage().

I didn't know that, thanks for the information Mark, i'm sorry for the misleading post. Till this point i was completely sure SendMessage would directly call the handler. Frown | :(

Guess i'm an idiot, but we have a saying here, "a good priest learns till death". Smile | :)

> The problem with computers is that they do what you tell them to do and not what you want them to do. <

Mark Salsbery26-Dec-08 11:27

Mark Salsbery

26-Dec-08 11:27

Heh no problem Smile | :)

I personally wouldn't be here if I didn't learn
stuff all the time.

This topic gets even trickier when you throw MFC into the mix, where some
operations may be thread safe, some aren't thread safe, and none are guaranteed
to always be thread safe. Your signature tag-line comes into play pretty quickly. Smile | :)

Cheers!
Mark

Mark Salsbery
Microsoft MVP - Visual C++
Java | [Coffee]

Code-o-mat26-Dec-08 10:38

cannot delete CStringArray like this? [modified]

26-Dec-08 10:38

Afaik CWnd::PostMessage doesn't do anything else but calls the API PostMessage specifying the handle of the window it represents, so there's not much difference between those two, however, if we stick to the general rule of not handing CWnd objects (pointers) over to different threads, i'd say, go for the api call. It's also somewhat safer since if your CWnd gets for some reason deleted then if a thread tries to call its CWnd::PostMessage it will give you the dreaded Access Voilation error. However, if you specify an invalid handle to the API PostMessage it will check that handle to see if a window with that handle exists and if not, then it will simply not post any message to it, thus, no crash. I have no idea about CWindow::PostMessage.
Alternatively you could use PostThreadMessage to post thread messages, these work almost the same way as window messages but they are not targeted at specific window procedures (their target hWnd will be NULL). If you use window messages, note, that in certain circumstances they can get lost. For example, when the user starts dragging a window in the GUI thread to move it around, a "custom" message loop starts up which filters out and throws away certain messages (i guess the reason for this is to avoid nasty situations like crashes, but ask Microsoft about that), so it could happen that while the user moves your window, the messages coming from your threads simply disappear in a black hole never to be seen again. However, this does not seem to happen with thread messages.

BobInNJ wrote:
I would also like to say that the codeproject has quickly become one of my favorite websites due to people like Code-o-mat.

Thanks, you are making me blush. Laugh | :laugh:

> The problem with computers is that they do what you tell them to do and not what you want them to do. <

kaviniswell25-Dec-08 15:58

25-Dec-08 15:58

hi,
these days i want to learn CStringArray and CTypedPtrList,so i creat a listbox for this:

/// .h ///

private:
    // to save all listitem string,like 
    // [listitem0:string1] [listitem0:string2]...  
    // [listitem1:string1] [listitem1:string2]...   
    CTypedPtrList < CPtrList,CStringArray* > m_ptrListAllListItem;

/// .cpp ///

// to add one listitem
void CListboxstrDlg::OnBtnAddlistitem() 
{
    static int sItemIndex = 0;
    CString str;
    str.Format("[listitem%d]",sItemIndex++);
    int nSel = m_list.AddString(str);
  
    CStringArray* pstrArry = new CStringArray [1];
    ASSERT(pstrArry != NULL);
    TRACE("------the new pstrArry addr is:%x \n",pstrArry);  // the addr is:3850cc
    pstrArry->Add(str);
    m_ptrListAllListItem.AddTail((CStringArray*)pstrArry);

    m_list.SetCurSel(m_list.GetCount()-1);
}

// to del one listitem
void CListboxstrDlg::OnBtnDellistitem() 
{
    int nSel = m_list.GetCurSel();
    if (LB_ERR == nSel)
    {
        return;
    }

    POSITION pos;
    pos = m_ptrListAllListItem.FindIndex(nSel);
    if (pos)
    {
        CStringArray* ptStrArry;
        ptStrArry = (CStringArray*)m_ptrListAllListItem.GetAt(pos);   
        TRACE("the ptStrArry addr is %x \n",ptStrArry); // the addr also is:3850cc
          
        m_ptrListAllListItem.RemoveAt(pos);
        delete ptStrArry;    // but this line debug info:
        // HEAP[listboxstr.exe]: Invalid Address specified to RtlValidateHeap( 00380000, 003850AC )        
        // :(  the CStringArray cannot delete like this? :confused:
        
        m_list.DeleteString(nSel);
        m_list.SetCurSel(nSel - 1);
    }    
}

thank u very much!!

modified on Thursday, December 25, 2008 10:07 PM

kaviniswell25-Dec-08 16:12

25-Dec-08 16:12

okay,i got it!

delete ptStrArry;
->
delete [] ptStrArry;

BobInNJ25-Dec-08 16:38

Re: cannot delete CStringArray like this? [modified]

25-Dec-08 16:38

I am looking at your post and the response you have already received. If the problem you are having is a compile time error then making the change already suggested is the way to go. However, from your description of the problem, it looks more like a run time problem. If so, making the change suggested is still a good idea but I doubt it would solve your problem.

What you are telling me is that you are allocated a valid pointer from new and then when you go to delete it, the delete fails. I cannot be 100% sure that you have not found a bug in delete but I am nearly 100% sure that you have not.

Typically delete does not return the space back to the operating system but instead puts the freed buffer on a linked list. This linked list can be corrupted in one of the following ways:
1) deleting the same pointer twice.
2) deleting space not allocated from new
3) indirecting a pointer that just happens to be pointed into this linked list.

Doing any of these can result in unexpected problems at run time. A variation on number 3 would be where you allocate a pointer of 10 bytes but then write to byte 11 or byte 12. This could also lead to the problem you are having.

I hope I have helped you and feel free to ask a follow up question.

Good Luck

kaviniswell25-Dec-08 17:31

25-Dec-08 17:31

thank u for your reply. i changed the codes like this:

delete ptStrArry;
-->delete [] ptStrArry;

this can resolve the question.

but when i test the codes, for example, run the function OnBtnLoop like this:

void CListboxstrDlg::OnBtnLoop() 
{
    TRACE("-------------start loop! \n");
    for (int i = 0; i < 10000; i++)
    {
        OnBtnAddlistitem();
    }
    for (i--; i >= 0; i--)
    {
        OnBtnDellistitem();
    }     
    TRACE("-------------finish loop! \n");
}

i found the memory used from 4668k to 4876k, and when i run the funcion again, the memory used form 4876k to 5384k. so i think the program must be something wrong...

but until now, i havenot found it..... Frown | :(

but another question

modified on Friday, December 26, 2008 7:24 AM

kaviniswell25-Dec-08 17:45

25-Dec-08 17:45

btw:
if i donot run the codes in the debug mode(Ctrl + F5), the memory used is normal...
so, why in debug mode(F5), the memory used from 4668k to 4876k,or from 4876k to 5384k?
i really donot know the reason... Frown | :(

anybody can explain this for me? thank u!

Richard Andrew x6426-Dec-08 0:40

Richard Andrew x64

26-Dec-08 0:40

In the debug version of the C++ runtime it allocates memory differently from the release version.

In the debug version, when you allocate new memory, the runtime also allocates a full page of extra memory before and after the buffer you have called for.

It fills these extra pages with the value CD. These are called guard pages. They are used for checking to see if your program has written outside the bounds of the memory you allocated.

This is why your debug version uses more memory than the release version.

kaviniswell26-Dec-08 1:23