Mutex not correctly used? Continuation of past que

问题:

I have a second question that is a continuation of this thread : How Does Windows Pick memory for threads?

void ThreadTest(LPTSTR* Pointer, mutex* MutexPtr)
{
    MutexPtr->lock();   
    wcout << *Pointer << endl;
    MutexPtr->unlock();
    return;
}
void FakeMain()
{
    mutex Mut;
    mutex* MutPtr = &Mut;
    LPTSTR Image = L"First String";
    LPTSTR* StrPointer = &Image;
    thread Test(ThreadTest, StrPointer, MutPtr);
    MutPtr->lock();
    if (true)
    {

        NewStringFunction(Image);
        MutPtr-unlock() // Added with the comment of Mike
        Test.join();        
        LPTSTR* StrPointer = &Image;        
        thread Test(ThreadTest, StrPointer, MutPtr);                
    }

    MutPtr->lock();
    Test.join();
    MutPtr->unlock();
    return;
};
void NewStringFunction(LPTSTR& Image)
{
    LPTSTR TempString = L"New String for new Thread";
    Image = TempString;
    return;
};

The Code above is emulates my code I am having issues with. The flow should be as follows:

• The FakeMain() initialized variables and sets a String;
• FakeMain then creates a thread for this string to be printed.
• New thread prints the string.
• The printing thread is destroyed and a new string is made by a separate external function.
• A new thread is made to print this new message

I have an "if" statement because the real code has a thread being made inside an if statement, i wanted to make sure the memory allocation represented this. The string is changed in a separate function because this is how the real code does this. I destroy the first thread because in the real code, the thread is displaying a window, which changes size, thus needs a new thread.

From writing this code I feel my mutexs are not implemented correctly. This code is not giving string errors but I think the mutex is being over written by the thread (i think). Watching the memory as the program runs, the first thread instance may over write the mutex, I am not sure. The last mutex.lock() in the main thread is throwing an error. Am I using mutexes correctly? Is there a cleaner way I should be locking/stopping threads?

回答1:

Don't use a mutex and don't share the memory.

Brute force approach:

All the sender needs to do is

Message msg = new Message(<important information>);
// send msg

And in the receiving code,

// use msg  
delete msg;

Very simple to write and use, but has a memory leak: if a message never gets serviced, it never gives back the RAM.

Better versions can get very complicated. Here is an outline for a simple one:

Create a pool of Messages
Assign all messages to a FreeList
Create a mutex to protect FreeList

FreeList is a list of messages that are not currently being used. When a Message needs to be sent, the first Message is removed from FreeList and handed to the caller. The caller fills out Message with the information to be sent and then sends the Message.

The receiver is responsible for returning the Message to FreeList .

Add a few functions to manage FreeList

GetMsg <changed to prevent collision with Windows GetMessage function>
    Message = NULL
    acquire mutex
    if FreeList not empty
        remove Message from FreeList
    release mutex
    return message            

ReturnMsg
    acquire mutex
    Add Message to FreeList
    release mutex

Sender is a bit more complicated

Message msg = GetMessage()
If msg not NULL
    msg.set(<important information>);
    std::thread temp(ThreadTest, msg);
    temp.detatch
else
    // log and handle <- can be simple like abort or complicated like recover lost Messages

And receiver is still pretty simple

// use msg  
ReturnMessage(msg);

This has a cap on the memory leak and can make it easy to track which Messages never got returned (in pool, but not in FreeList) so you can attempt to track down which jobs got blocked or blown up. Fixing the lock or blow-up is another problem.

OP had a good idea that I think I have about right. Here are the guts of a basic thread function. It hangs out in the background running until sent a terminate message. It blocks waiting for a message, consumes the message, and then puts it back in FreeList and blocks until another message arrives.

void threadfunc()
{
    MSG winmsg;
    BOOL rval;

    while (GetMessage(&winmsg, (HWND__ *) -1, 0, 0) != -1)
    {
        Message * msg = (Message *)winmsg.wParam;
        // do stuff with msg
        ReturnMsg(msg);
    }
    // GetMessage failed. Find out why and try to recover or die gracefully
}

It needs to be supported by something like

bool PostMsg(<Important Information>,
             DWORD & BackgroundThreadId)
{
    Message * msg = GetMsg();
    if (msg != NULL)
    {
         msg.set(<Important Information>)
        if (PostThreadMessage(BackgroundThreadId,
                              WM_USER,
                              (WPARAM) msg,
                              0); != 0)
        {
            return true;
        }
        else
        {
            // failed. Find out why and try to recover or die gracefully
            ReturnMsg(msg);
            return false;
        }
    }
    else
    {
        // out of free messages. Try to find out why
    }
}

PostMsg feeds Messages into the background thread's message queue. Messages come from the FreeList and only the FreeList needs mutex protection. More than one thread can be in use. I don't think this can be done with std::thread since std::thread doesn't give access to the underlying thread handle (Just checked, it can be done) or the thread's message queue. It'as all based on Windows calls anyway, so forget about portability.

回答2:

The lock call before the if statement does not have a matching unlock. Also you have a potential deadlock if the main thread gets the mutex before the test thead. Also your last join is waiting on the wrong thread. As far as I can tell there is no reason to lock the mutex to do the join. Just do the join.