MFC passing parameters to threads, where parameters are writable & shared between main dialog and thread

Question

I have this code:

// ------- My dialog header file ----------
class CMexifierDlg : public CDialogEx {
public:
    unsigned int static CMexifierDlg::doit(LPVOID pParam); // this is the thread
    CMexifierDlg* pMexifierDlg=this;
    int x, // used to initialize the dialog AND by thread
    y,   //   set by dialog,  used to pass info to thread
    z    // used by MULTIPLE thread functions, but NOT by dialog
};

// ------- My CPP file -----
void CMexifierDlg::OnClickedCheck1() {
    CMexifierDlg* myparm; myparm = pMexifierDlg;
    AfxBeginThread(doit, (LPVOID)myparm); 
        // thread terminates itself when job is done
}
// -----  my thread function
unsigned int   CMexifierDlg::doit(LPVOID pParam) {
    // this is where the actual work happens
    CMexifierDlg* D_ptr=(CMexifierDlg*)pParam;
    int y = x;  // doesn’t compile
    int y = D_ptr->x;   // fails at runtime
    // now we make many calls (as part of thread)
    // to other functions that need D_ptr
    return 0;   // thread completed successfully
}

The header file has MANY variables, some of which are needed ONLY by my main (and only) dialog, some of which are shared by the dialog and the ONE thread I create from the dialog when a button is pressed and some that are used ONLY by many functions called within the thread. Once the thread starts, it runs to completion before ANY other actions will be allowed in the dialog, except exiting the whole program. However, once the thread is done, it can be restarted with NEW data from the dialog. Can someone SHOW me what I have wrong that will make this work? I am getting lost with understanding HOW to pass the correct thing to my thread. One thing I tried is to MOVE my thread-only var definitions to the .CPP file as global data and that SEEMS to be OK (but is it a proper thing to do)?

Answer 1

You obviously can't do

int y= x;   // doesn’t compile

as your thread function is static and has no class instance member variables.

Your

int y= D_ptr->x;    // fails at runtime

Looks OK. I've created a similar code in my test project and it works fine.

Your issue ("fails at runtime") must be elsewhere.

UPDATE:

Here is that ASSERT from MFC\wincore.cpp line 970:

void CWnd::AssertValid() const
{
    ...
    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.

Please note

...and have used that object in a way that was not intended.

The code you posted does NOT use that object in a "bad" way.

As I suggested, the issue is elsewhere. But, as you said, you do need to pass a window handle to the thread, if you want to use it as a window.

Answer 2

One little code review in advance:

Don't initialize pMexifierDlg in the class declaration. A better place would be to do it in the constructor:

CMexifierDlg* m_pMexifierDlg;  // also prefix members with m_ or something to make clear in the code that it's a class member

void CMexifierDlg::CMexifierDlg()
{
    m_pMexifierDlg = this;
}

But I'd omit the m_pMexifierDlg step completely because this is omnipresent in member function code anyway. Just have it like this:

AfxBeginThread(doit, (LPVOID)this);

So you also can remove the unnecessary myparm assignment.

Having said this, I think it's desirable to have the worker thread running as a separate entity outside the class. For example, the worker won't exit if you close or destroy the dialog. It's always a good idea to separate gui code from your logic code and even if you have one dialog and only one thread at a time that is associated with it somehow, you might run into pitfalls if you do gui things directly from the worker thread. Manipulating atomic variables like ints might work but use CMutex for more complex things like CString. But I really urge you to have a clear protocol for communication between the two threads, as already mentioned in your recent question.

If you want to have the worker thread in a class, create a separate class in addition to CMexifierDlg.

I recommend you have a closer look into instantiating C++ classes, lifetime of the this pointer, and win32/MFC gui thread <-> worker thread communication.