MFC - Cdialog::OnOK() in Thread

Hi,

I've a problem, which i don't know how to solve.

I've a class LoginDlg, which is defined this way

Code:

---

class LoginDlg : public CDialog

---

Dialog has one button, which has method

Code:

---

void LoginDlg::OnBnClickedOk()

---

In this method a I create a Thread

Code:

---

pLoginThread = AfxBeginThread(LogIn, myUserInfo, THREAD_PRIORITY_NORMAL);

---

I want to call OnOK() method after pLoginThread finish its job. Part of code is:

Code:

---

UINT LogIn(LPVOID lParam) {
  // Any code... (Downloading file)
  return 1;
}
void LoginDlg::OnBnClickedOk() {
  CWinThread* pLoginThread = AfxBeginThread(LogIn, myUserInfo, THREAD_PRIORITY_NORMAL);
  // If i write OnOK() here, it doesn't wait on finishing thread
}

---

I really don't know, how to do it. So please, help me :)

you can call a method instead of creating a thread, is there any specific reason to do that??

Yes, there is a reason.

If I would call a method instead of creating thread, Application would be frozen for a while.

One solution is post an user message from the thread function and call OnOK from its handler function.
See next example:

Code:

---

// TestOKDlg.h
#define WM_DO_END_DIALOG (WM_APP + 1)

class CTestOKDlg : public CDialog
{
    //...
    afx_msg LRESULT OnDoEndDialog(WPARAM wParam, LPARAM lParam);
};

---

Code:

---

// TestOKDlg.cpp
BEGIN_MESSAGE_MAP(CTestOKDlg, CDialog)
// ...
    ON_MESSAGE(WM_DO_END_DIALOG, OnDoEndDialog)
END_MESSAGE_MAP()
//...
UINT AFX_CDECL LogIn(LPVOID lpParam)
{
    // ...some very long task...
    ::PostMessage((HWND)lpParam, WM_DO_END_DIALOG, 0, 0);
    return 0;
}

void CTestOKDlg::OnBnClickedOk()
{
    AfxBeginThread(LogIn, m_hWnd, THREAD_PRIORITY_NORMAL);
}

LRESULT CTestOKDlg::OnDoEndDialog(WPARAM wParam, LPARAM lParam)
{
    OnOK();
    return 0;
}

---

I'm not sure what you are trying to do with the Dialog box. It sounds like you want a screen on the app which is active while downloading a file, or you want to continue using the dialog box while downloading a file.

Anyhow, if your dialog box is modal, you are going be stuck with in dialog box while downloading.


If I wanted to continue using the app while download, I would start thread and send it a pointer to the Dialog box once you started it, then execute OnOK() within the thread when it's done downloading:

Code:

---

AfxBeginThread(LogIn, myUserInfo, THREAD_PRIORITY_NORMAL, LoginDlg  *pDlg ){

 ... dosomething

AfxMessageBox("Thread Done, Closing Dialog Box");
pDlg->OnOk();

....

pDlg->CallanyFunctionIntheDialogBox();
pDlg->AccessAnyMemberInDialog;
pDlg->DoAnythingToTheDialogBoxorItItsParentorParentVars;
pDlg->I_Own_You;




};


 AfxBeginThread(......., ...., this );

---

...If I wanted to use a dialog box/app until the download is done, That's how I would do it, ..... but, if that where the case a Modless Dialog would do the job better called with in the App/Dialog/CFormView.

Or you create your dialog box as Modless box, which the Thread closes when it is done.

If you want to start a thread and update a dialog box/app during certain points in the thread, I would think you have to pass a pointer to the DialogBox/App/View/Docuement, .... etc.

Try passing a pointer to the dialog box in your thread. I haven't done it, but I pass pointers to other dialogboxes from other dialog boxes all the time. In my apps I have about 20 dialog boxes which I can access any button, function, variable, etc at anytime.

...I havn't done much with threads, but I think it should work.

2 Bald3rr and ADSOFT:
the only acceptable solution is the one provided by ovidiucucu.
Some more information about using working threads you could find here

Quote:

---

Originally Posted by VictorN

2 Bald3rr and ADSOFT:
the only acceptable solution is the one provided by ovidiucucu.
Some more information about using working threads you could find here

---

Maybe accepatable to you, but I wouldn't do it that way.

Quote:

---

Originally Posted by ADSOFT

Maybe accepatable to you, but I wouldn't do it that way.

---

Then you will be in trouble!:sick:

Thanks everyone, especially to ovidiucucu. It's working fine.

Quote:

---

Originally Posted by VictorN

Then you will be in trouble!:sick:

---

I doubt it, I bet I can get a thread to work that that I pass a pointer(to the thread function that is) to a dialog box or better yet a pointer to my View Class with out having to write all that Code the OV did. With a pointer to my View Class I can control anything in View Class, it's related Dialog Boxes and the CDocument Class, and probobly the whole App. Heck the thread could be a monitor to the app makeing sure that the user hasn't done anything to violate integrity or I could update the Application or it's Database without the user knowing it, do backups, ....etc, while working out of just one thread.


That's cool if OV wants to do it with all that code, to each his own! However; ...."There's more than one way to skin a cat!" I would rather just pass one pointer and go from there.

Quote:

---

Originally Posted by ADSOFT

With a pointer to my View Class I can control anything in View Class, it's related Dialog Boxes and the CDocument Class, and probobly the whole App.

---

The bottom line is that any shared data needs to be protected when accessed from multiple threads. When you simply pass a view pointer to the thread and access MFC controls within the thread, you aren't synchronizing data access properly and it will lead to race conditions.

Quote:

---

Originally Posted by Arjay

The bottom line is that any shared data needs to be protected when accessed from multiple threads. When you simply pass a view pointer to the thread and access MFC controls within the thread, you aren't synchronizing data access properly and it will lead to race conditions.

---

Well I can tell you know about threads, and when it comes to data synchronizing it's all how you write the thread.


The OP wanted to access a control from within a thread, I just took it to another level. Data synchronization is not just related to threads, it must also be taken into consideration at the application level.


The general topic here is access to controls from a worker threads. :D

Quote:

---

Originally Posted by ADSOFT

The OP wanted to access a control from within a thread, I just took it to another level. Data synchronization is not just related to threads, it must also be taken into consideration at the application level.

---

Bottom line is that you shouldn't access MFC controls from a secondary thread.

Sure, you can pass a pointer of an MFC object to a secondary thread, but if you need to access MFC class functionality from the pointer, you'll need to provide your own synchronization. You can do that using the method that Ovidiu and Victor were talking about or you can create thread safe accessor methods.

Quote:

---

Originally Posted by Arjay

Bottom line is that you shouldn't access MFC controls from a secondary thread.

Sure, you can pass a pointer of an MFC object to a secondary thread, but if you need to access MFC class functionality from the pointer, you'll need to provide your own synchronization. You can do that using the method that Ovidiu and Victor were talking about or you can create thread safe accessor methods.

---

Ok, well I'm just getting into threads and it seems that you have tons of experience with them.

.... here's what I've ended up with, I'm hungry so I have to stop for a while(maybe you will respond while I get some dinner); I sent the threading function a this pointer and in the worker thread recast it back to a CFormview which has tons of buttons. It works but I keep getting the following Assertion:

HTML Code:

---

        CObject* p;
                ASSERT((p = pMap->LookupPermanent(m_hWnd)) != NULL ||
                        (p = pMap->LookupTemporary(m_hWnd)) != NULL);
                ASSERT((CWnd*)p == this);  // must be us

                // Note: if either of the above asserts fire and you are
                // writing a multithreaded application, it is likely that
                // you have passed a C++ object from one thread to another
                // and have used that object in a way that was not intended.
                // (only simple inline wrapper functions should be used)
                //
                // In general, CWnd objects should be passed by HWND from
                // one thread to another.  The receiving thread can wrap
                // the HWND with a CWnd object by using CWnd::FromHandle.
                //
                // It is dangerous to pass C++ objects from one thread to
                // another, unless the objects are designed to be used in
                // such a manner.

---

What's a "simple inline wrapper function"? that's probobly why CWD::FormHandle didn't work.


I understand that the SendMessage via m_hwnd approach works, however it seems like Microsoft is implying to use simple wrapper function with CWD::FormHandle .... :D

This is why you can't usually pass MFC objects to another thread. The simple wrapper objects that the MFC comments are referring to are the lightweight inlined functions that MFC uses to send a message to a control. Anything that causes MFC to walk its internal maps or access more complex functionality will be likely to fail.

At any rate, take a look at the sample code in the post: Sharing a thread safe std::queue between threads w/progress bar updating

This sample includes code that shares a thread safe std::queue between two worker threads and the main UI thread. The worker threads push items onto the queue (with each worker thread pushing the items on at different rates). The UI thread gets signalled when an item is available in the queue via PostMessage and a user defined message. The pushing the items on the queue and popping them off the queue are done in a thread safe manner (using a critical section lock). The threading and queue access is wrapped into a progress manager class so that the thread management and queue synchronization is encapsulated within the class.

This approach makes a clean interface between the MFC UI code and the thread code. It makes debugging easier because the UI logic (enabling/disabling buttons, changing state, etc.) can be worked out separately from the threading code. Likewise, the threading code can be debugged separately from the UI code (by calling the methods of the progress class directly).

Tying this back to the post's related topics, you'll notice I pass in a hWnd from the dialog in the CProgressMgr::Start method. This hWnd lets the threads know where to send the queue notification messages.

Start (et al) button handler code:

Code:

---

//+------------------------------------------------------
// Start, pause, and resume button handler
//+------------------------------------------------------
void CStartStopDlg::OnBnClickedStartPauseResume()
{
        m_btnStartPauseResume.EnableWindow( FALSE );
        m_btnStop.EnableWindow( FALSE );

        switch( ToggleSPRState( ) )
        {
        case TS_START:
                // Remove the list control items               
                m_ctrlList.DeleteAllItems( );
                m_uListItemCount = 0;

                // Start the threads
                m_ProgressMgr.Start( GetSafeHwnd( ) );
                break;
        case TS_PAUSE:
                // Pause the thread
                m_ProgressMgr.Pause( );
                break;
        case TS_RESUME:
                // Resume the thread
                m_ProgressMgr.Resume( );
                m_ThreadState = TS_START;
                break;
        default:
                ASSERT( 0 ); // We shouldn't reach this
        }

        m_btnStartPauseResume.EnableWindow( TRUE );
        m_btnStop.EnableWindow( TRUE );
}

---

WM_USER_INC_PROGRESS handler code. This message gets sent by the secondary thread to notify the main thread that a new item has been pushed on the queue. Sorry about using the magic numbers in the case statements. They represent T1 and T2 (but my bad for using magic numbers).

Code:

---

//+------------------------------------------------------
// OnIncProgress handler. Receives user defined message
// posted from the secondary thread. When we receive this
// message we increment the progress bar and add the item
// to the list control.
//+------------------------------------------------------
LRESULT CStartStopDlg::OnIncProgress( WPARAM wParam, LPARAM lParam )
{
        UINT uID = (UINT) lParam;

        if( TS_START != m_ThreadState ) return 0;

        // Increment the progress controls
        switch( uID )
        {
        case 1:
                m_ctrlProgress1.StepIt( );
                break;
        case 2:
                m_ctrlProgress2.StepIt( );
                break;
        }

        // Get the item out of the queue and put it in the list control
        InsertItemIntoListView( );

        return 1;
}

---

Let's look at the InsertItemIntoListView( ) method referenced above. This method accesses the first item in the queue, inserts it into the listview control and then deletes it from the queue. You'll notice that there doesn't appear to be any thread synchronization (because it's encapsulated entirely within the CProgressMgr thread).

Code:

---

//+------------------------------------------------------
// Called when a notification message has been received from
// one of the secondary threads. Removes one or more items
// from the shared queue in a thread safe manner and inserts
// the item into the list control.
//+------------------------------------------------------
void CStartStopDlg::InsertItemIntoListView( )
{
        CItem* pItem = NULL;

        CString sCount;
        CString sThreadID;
        CString sThreadCount;

        // Retrieve the next queue item (thread-safe method)
        while( NULL != ( pItem = m_ProgressMgr.GetNextItem( ) ) )
        {
                // Format the listview column strings
                sCount.Format( _T( "%d" ), m_uListItemCount );
                sThreadID.Format( _T( "This is an item from thread: %d" ), pItem->GetID( ) );
                sThreadCount.Format( _T( "%d" ), pItem->GetCount( ) );

                // Insert the item into the list view and ensure the item is visible
                m_ctrlList.InsertItem( m_uListItemCount, sCount );
                m_ctrlList.SetItemText( m_uListItemCount, 1, sThreadID );
                m_ctrlList.SetItemText( m_uListItemCount, 2, sThreadCount );
                m_ctrlList.EnsureVisible( m_uListItemCount, TRUE );

                // Remove this item from the queue (thread-safe method)
                m_ProgressMgr.PopItem( );

                m_uListItemCount++;
    }
}

---

Let's take a look at the thread safe queue accessor methods. There are 3 of them: The GetNextItem and PopItem methods that are accessed by the UI thread and the PushItem method which is accessed by the secondary threads.

Code:

---

    // Gets the next item from the ItemQueue
CItem* GetNextItem( )
{
        CAutoLockT< CItemQueue > lock(&m_ItemQueue);
   
        if( TRUE != m_ItemQueue.empty( ) )
        {
                return m_ItemQueue.front( );
        }

        return NULL;
}

// Removes the front item from the ItemQueue
void PopItem( )
{
        CAutoLockT< CItemQueue > lock( &m_ItemQueue );

        if( TRUE != m_ItemQueue.empty( ) )
        {
                delete m_ItemQueue.front( );
                m_ItemQueue.pop( );
        }
}

HRESULT PushItem( CItem* pItem )
{
        HRESULT hr = S_OK;

        CAutoLockT< CItemQueue > lock( &m_ItemQueue );

        try
        {
                m_ItemQueue.push( pItem );
        }
        catch( std::bad_alloc )
        {
                hr = E_OUTOFMEMORY;
        }               

        return hr;
}

---

The thread safety of these methods may not be obvious, but they are handled by the CAutoLockT class which is locking a wrapper class to a
critical section object. This class simply obtains a critical section lock (EnterCriticalSection(...)) in its constructor and releases the lock (LeaveCriticalSection(...)) in its destructor which gets called when the CAutoLockT instance goes out of scope. The single line of code that gets this done ensures the queue is only ever accessed by one thread at a time.

Well I'll have to dig through your Ariticle. For all the details


The worker function is as follows and it do seem to get started, but the button that it calls does a lot of work, and eventuall I get the Assertions:

Code:

---

UINT WorkerThreadProc( LPVOID Param ){

        CFormHdrView *pCFV ;
        pCFV  = (CFormHdrView *)Param;

        if(::gpCFVw_Hdr == NULL)::AfxMessageBox("Null Pointer");

       
        pCFV->OnButtonTotlsRevs();

        return TRUE;


}

---

That looks pretty light weight to me. I don't know if forcing the function Inline would help the thread cut further into my CFormView App with tons of CDialogs.

I guess when the manufacture of a product, which in this case is Microsoft, says the product is not intended to be used in a certain way, that pretty much draws the line. Accessing the GUI Objects from threads was "not the way the Objects were intended to used" ... that's a wrap.

It doesn't matter if it looks lightweight - what matters is that the OnButtonTtlsRevs( ) method is called in the context of the secondary thread which is not allowed because you are calling MFC objects from there.

The simplest way to solve this is to follow Ovidiu's advice and post a user defined message.

First declare the user defined message (in stdafx.h)

Code:

---

#define WM_USER_TTLS_REVS WM_USER + 1

---

Next declare a message handler in the view class and call your existing OnButtonTotlsRevs() method.

Code:

---

afx_msg LRESULT OnTtlsRevs( WPARAM wParam, LPARAM lParam );

LRESULT CFormHdrView::OnTtlsRevs( WPARAM wParam, LPARAM lParam )
{
  OnButtonTotlsRevs( );
}

---

Be sure to add the appropriate Message map entry for the user defined message

Code:

---

BEGIN_MESSAGE_MAP(CFormHdrView, CFormView)
        ON_MESSAGE( WM_USER_TTLS_REVS, OnTtlsRevs )
END_MESSAGE_MAP()

---

Then modify the thread code to pass in the hWnd of the view class (use the GetSafeHWnd( ) method)....and post a message to the view inside the thread.

Code:

---

UINT WorkerThreadProc( LPVOID lParam )
{
  HWND hWnd = (HWND)lParam;

  ASSERT( NULL != hWnd );

  // Pause a bit before sending the message
  Sleep( 10000 );

  // Send a message to the form view and cause it to click
  // on the button
  ::PostMessage( hWnd, WM_USER_TTLS_REVS, 0, 0 );

  // Pause here for a bit so the thread doesn't close
  Sleep( 10000 );

  return TRUE;
}

---

P.S. I through in a few sleep statements in the thread so things wouldn't happen to fast. Generally you wouldn't want these in a real app.

I got that to work without the Assertion Messages, thanks.

Quote:

---

Originally Posted by ADSOFT

I got that to work without the Assertion Messages, thanks.

---

Your'e a genius. Sometimes even the wrong solution seem to work. Someday somebody will make a seemingly unimportant change to your form class and everything will blow up.
You seem not to understand that when a message handler in a class is invoked from another thread this can happen anywhere in the other threads context but if the other thread just posts a message to that thread it will be processed in a way that synchronisation is not even necessary.
Kurt

Quote:

---

Originally Posted by ZuK

Your'e a genius. Sometimes even the wrong solution seem to work. Someday somebody will make a seemingly unimportant change to your form class and everything will blow up.
You seem not to understand that when a message handler in a class is invoked from another thread this can happen anywhere in the other threads context but if the other thread just posts a message to that thread it will be processed in a way that synchronisation is not even necessary.
Kurt

---

Well, maybe you should learn how to read, or read all the posts before you put in your two centavos, or maybe you should spend some time on this continent so you can understand English a little bit better: I got Arjay's solution to work without the Assertion messages that I was getting by passing a CFormView pointer: Does that clarify things?

Now go back and read my previous post, and all the posts before that, ok!!

Btw, Thanks again to Arjay.:)


If I ever get my CFormView Class syncronized, I'm going to go to Austria, and take Arnold with me, just to make sure it works over there and they know how to use it. .... then I will get Arnold S. (You know the Terminator) to Email Mr. Kurt a.k.a Zuk, so he understands every little detail.

Hasta la Vista Baby :wave:

Quote:

---

Originally Posted by ADSOFT

Code:

---

UINT WorkerThreadProc( LPVOID Param ){

        CFormHdrView *pCFV ;
        pCFV  = (CFormHdrView *)Param;

        if(::gpCFVw_Hdr == NULL)::AfxMessageBox("Null Pointer");
        ...
}

---

---

Again it is wrong. :eek:
Don't pop up any window from within a worker thread! The illusion it sometimes "works" does not mean it is correct and will work for ever. Unfortunately no, it won't!

Quote:

---

Originally Posted by VictorN

Again it is wrong. :eek:
Don't pop up any window from within a worker thread! The illusion it sometimes "works" does not mean it is correct and will work for ever. Unfortunately no, it won't!

---

Well the problem is that you don't know how to explain yourself. If you would have pinpointed documentation stating that MFC obects should not be called from within threads, then I would have believed you. .... fortunately I found documention that states that.

Code:

---

CObject* p;
                ASSERT((p = pMap->LookupPermanent(m_hWnd)) != NULL ||
                        (p = pMap->LookupTemporary(m_hWnd)) != NULL);
                ASSERT((CWnd*)p == this);  // must be us
                // Note: if either of the above asserts fire and you are
                // writing a multithreaded application, it is likely that
                // you have passed a C++ object from one thread to another
                // and have used that object in a way that was not intended.
                // (only simple inline wrapper functions should be used)
                //
                // In general, CWnd objects should be passed by HWND from
                // one thread to another.  The receiving thread can wrap
                // the HWND with a CWnd object by using CWnd::FromHandle.
                //
                // It is dangerous to pass C++ objects from one thread to
                // another, unless the objects are designed to be used in
                // such a manner.

---

Note where it says, ".... must be us". It's not my fault that Microsoft's objects can't run from threads.

I theory you should be able access any window from a global pointer, .... that's what Microsoft admits, when it says "..... it mus be us".

Instead of saying what should be done and not be done, why don't you quote the manufacture, i.e, Microsoft on the do's and don't of threads and MFC. You sound like your making the stuff up. Next time quote Microsoft on the limitation of Microsofts' products, not 3rd and 4rth party websites.:thumbd:

Quote:

---

Originally Posted by VictorN

Again it is wrong. :eek:
Don't pop up any window from within a worker thread! The illusion it sometimes "works" does not mean it is correct and will work for ever. Unfortunately no, it won't!

---

Look this is what you should have done from the very begining:


http://support.microsoft.com/kb/147578

Quote:

---

Originally Posted by ADSOFT

Look this is what you should have done from the very begining:


http://support.microsoft.com/kb/147578

---

I have learned this hard way. Calling any MFC stuff that involves any GUI will surely fail sooner or later. I have spent weeks debugging and didn't understand why my application is throwing asserts.
The right approach is using GUI threads: sending message from dialog to thread and back as someone pointed that in this thread. Just derive your thread from CWinThread.
Sending data to thread is available via PostThreadMessage function.

Quote:

---

Originally Posted by streamer

I have learned this hard way. Calling any MFC stuff that involves any GUI will surely fail sooner or later. I have spent weeks debugging and didn't understand why my application is throwing asserts.
The right approach is using GUI threads: sending message from dialog to thread and back as someone pointed that in this thread. Just derive your thread from CWinThread.
Sending data to thread is available via PostThreadMessage function.

---

This is a common approach, however, I prefer to not use messaging to transfer data between threads. I simply put any data that needs to be shared between threads in a class, expose the data via thread safe accessor methods, and then pass a pointer of the class to the threads. I use user defined messages, but these are only used by worker threads to signal the UI thread that the data has changed (whereby the UI thread gets the data from the thread safe accessor methods). I find that this approach is cleaner and requires fewer message handlers.