Overview

I need to save a CDocument in a background worker thread. There is a point in our MFC application which prompts the user to save before continuing. Normally, they are able to continue without saving, and there is no problem. However, occasionally, we need that document later in the process, so if the user clicks "No", we want to save a temp version of the file in the background without making the user wait for the save to continue.

Problem

When I launch AfxBeginThread(SaveDocumentThread, &threadInput) the &threadinput has been cleared from memory before the SaveDocumentThread starts.

Code

BOOL SPackagerDoc::OnSaveDocument( IN LPCTSTR lpszPathName)
{
    ProcessDocumentThreadInput threadInput(this, lpszPathName);
    
    // Temp Save Mode
    if (m_bTempMode)
    {
        m_TempSaveThread = AfxBeginThread(SaveDocumentThread, &threadInput);
        // This fixes the problem, but is considered unstable
        // if (m_TempSaveThread->m_hThread)
        //        WaitForSingleObject(m_TempSaveThread->m_hThread, 500);
        return TRUE;
    }

    // Normal save mode
    SFileLoadingDialog loadingDialog(SFileLoadingDialog::SAVE, lpszPathName, SaveDocumentThread, &threadInput);
    BOOL result = (BOOL)loadingDialog.DoModal();

    return result;
} 

StUInt32 SPackagerDoc::SaveDocumentThread(IN StVoid* pParam)
{
    ProcessDocumentThreadInput* input = (ProcessDocumentThreadInput*)pParam;
    ASSERT_NOT_NULL(input);
    ASSERT_NOT_NULL(input->pPackager);
    ASSERT_NOT_NULL(input->pszPathName);

    CString path_name(input->pszPathName);

    BOOL result = input->pPackager->SPackagerDocBase::OnSaveDocument(path_name);
    return result;
}

If I uncommend WaitForSingleObject(..., 500); then the thread starts, all the information is present, and there are no errors. But if I remove those lines then in SaveDocumentThread input is NULL and all data is zeros or garbage.

Is there a way to ensure the SaveDocumentThread has started before moving on. IE, wait for thread to start, but not for a specified amount of time (500 ms). It may be that 500 ms is not a sufficient wait time on some other computers.

Is there an "official" way to do this?

CodePudding user response：

This is the issue of the scope of variable. Following comments specified the scope of local variable threadInput.

    ProcessDocumentThreadInput threadInput(this, lpszPathName); // <=== threadInput created
    
    if (m_bTempMode)
    {
        m_TempSaveThread = AfxBeginThread(SaveDocumentThread, &threadInput);
        // This fixes the problem, but is considered unstable
        // if (m_TempSaveThread->m_hThread)
        //        WaitForSingleObject(m_TempSaveThread->m_hThread, 500);
        return TRUE; // <=== threadInput destructed
    }

WaitForSingleObject delays the destruction of the variable threadInput and you see the result.

To overcome the scope of local variable.

1. Store it in a class member variable.
2. Store it as a (better be smart) pointer and (better not) handle it's destruction.

CodePudding user response：

As others have pointed out, the problem is the lifetime of threadInput ends before the thread begins.

You can dynamically allocate the instance of ProcessDocumentThreadInput and pass the pointer to that instance to the thread.

auto* threadInput = new ProcessDocumentThreadInput(this, lpszPathName);
...
AfxBeginThread(SaveDocumentThread, threadInput);

However, in this case, the responsibility to release the memory gets messy. Since you put C 11 tag in your question, you might want to make use of std::shared_ptr or std::unique_ptr and pass it to the thread, which would land you in using std::thread instead of AfxBeginThread. (BTW, I have no experience using MFC.)

BOOL SPackagerDoc::OnSaveDocument( IN LPCTSTR lpszPathName)
{
    ...
    std::thread t(SaveDocumentThread, std::make_unique<ProcessDocumentThreadInput>(this, lpszPathName));
    ...
}

...

StUInt32 SaveDocumentThread(std::unique_ptr<ProcessDocumentThreadInput>&& threadInput)
{
   ...
}