Multithreading issue while using Mutex in SerialCommunication C++

Question

I am developing a serial communication software using windows functions. In this CSerialCommhelper is class which handles all the serial communication functions and CphysicalLayer is one which make use of that class.

class CSerialCommHelper :
public CCommAgent
 {
    HANDLE m_pPortHandle;           //Handle to the COM port
    HANDLE m_hReadThread;           //Handle to the Read thread
    HANDLE m_hPortMutex;            //Handle to Port Mutex
    std::wstring m_strPortName;     //Portname
    COMMTIMEOUTS m_CommTimeouts;    //Communication Timeout Structure
    _DCB dcb;                       //Device Control Block
    DWORD m_dwThreadID;     //Thread ID

public:
    CSerialCommHelper(CPhysicalLayer *);
    virtual HRESULT Open();
    virtual HRESULT ConfigPort();
    static void * ReadThread(void *);
    virtual HRESULT Write(const unsigned char *,DWORD);
    virtual HRESULT Close();
    //virtual HRESULT Flush(DWORD dwFlag = PURGE_TXCLEAR | PURGE_RXCLEAR);
    wstring StringToWstring(const string &);
    ~CSerialCommHelper(void);
};

CommAgent is contains a CphysicalLayer pointer for notify the physicalLayer when data is received.

HRESULT CSerialCommHelper::Write(const unsigned char *pucDataToWrite,DWORD ulLength)
{
    unsigned long  bytesWritten=0, ij = 0;
    WaitForSingleObject(m_hPortMutex,INFINITE);
    if(WriteFile(m_pPortHandle,pucDataToWrite,ulLength,&bytesWritten,NULL))
    {
    if(!ReleaseMutex(m_hPortMutex))
    {
        DWORD err=GetLastError();

        // Mutex released succesfully..
    }
    }
    if (bytesWritten != ulLength)
            return E_FAIL;
    return S_OK;

}
void * CSerialCommHelper::ReadThread(void * pObj)
{
    CSerialCommHelper *pCSerialCommHelper =(CSerialCommHelper *)pObj; 
    DWORD dwBytesTransferred = 0;
    unsigned char byte = 0;

    while (pCSerialCommHelper->m_pPortHandle != INVALID_HANDLE_VALUE)
    {
        pCSerialCommHelper->m_strBuffer.clear();
        pCSerialCommHelper->m_usBufSize=0;
        WaitForSingleObject(pCSerialCommHelper->m_hPortMutex,INFINITE);
        do
        {
            dwBytesTransferred = 0;
            ReadFile (pCSerialCommHelper->m_pPortHandle, &byte, 1, &dwBytesTransferred, 0);
            if (dwBytesTransferred == 1)
            {
                pCSerialCommHelper->m_strBuffer.push_back(byte);
                pCSerialCommHelper->m_usBufSize++;
                continue;

            }
        }
        while (dwBytesTransferred == 1);
        if(pCSerialCommHelper->m_usBufSize!=0)
        {
            CProtocolPacket *pCProtocolPacket = new CProtocolPacket(0,2048);
            pCProtocolPacket->AddBody(pCSerialCommHelper->m_usBufSize,(unsigned char*)pCSerialCommHelper->m_strBuffer.c_str());
            pCSerialCommHelper->m_pCPhysicalLayer->Data_ind(pCProtocolPacket);
            delete pCProtocolPacket;
        }
            ReleaseMutex(pCSerialCommHelper->m_hPortMutex);
        Sleep(2);
    }
    ExitThread(0);


    return 0;
}

This is how i am creating file and mutex

    m_pPortHandle = CreateFile(m_strPortName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL,
                         OPEN_EXISTING,NULL, NULL );
if (m_pPortHandle == INVALID_HANDLE_VALUE)

    {
        return E_HANDLE;
        //throw failure
    }

m_hPortMutex = CreateMutex(NULL,TRUE,L"MY_MUTEX");


if( m_hReadThread = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)ReadThread,(LPVOID)this,0,&m_dwThreadID))
{
}
else
{
    return E_FAIL;
}
return S_OK;

But after writing a string to the port the write function is releasing mutex successfully, but read thread is still waiting.

Answer 1

The problem is with Readfile function in ReadThread .It is not returning properly. I don't know the reason.

void * CSerialCommHelper::ReadThread(void * pObj)
{
    CSerialCommHelper *pCSerialCommHelper =(CSerialCommHelper *)pObj; 
    DWORD dwBytesTransferred =0;
    char byte[1];
    string buffer;

    while (pCSerialCommHelper->m_pPortHandle != INVALID_HANDLE_VALUE)
    {

        WaitForSingleObject(pCSerialCommHelper->m_hPortMutex,INFINITE);
        do
        {
           dwBytesTransferred = 0;
                bool bReadResult= false;

                 bReadResult = ReadFile (pCSerialCommHelper->m_pPortHandle,byte,1,&dwBytesTransferred,NULL);


                if (dwBytesTransferred == 1)
                {
                    buffer.push_back(byte[0]);
                    continue;

                }


        }
        while (dwBytesTransferred == 1);

            pCSerialCommHelper->m_pCPhysicalLayer->Data_ind((unsigned char *)buffer.c_str());
            ReleaseMutex(pCSerialCommHelper->m_hPortMutex);

    }
    ExitThread(0);


    return 0;
}

I tried replacing the code with this snippet and found that bReadResult is not getting updated properly.

Answer 2

When you create the mutex, you set the bInitialOwner argument to TRUE. So at this point the mutex is owned by the main thread.

m_hPortMutex = CreateMutex(NULL,TRUE,L"MY_MUTEX");

You then create a ReadThread which attempts to acquire the mutex. This is obviously going to block until the main thread releases it.

WaitForSingleObject(pCSerialCommHelper->m_hPortMutex,INFINITE);

When the main thread attempts to write something, the first thing it does is attempt to acquire the mutex again.

WaitForSingleObject(m_hPortMutex,INFINITE);

Since the main thread already owns the mutex, this call will return immediately without blocking, but at this stage you've now acquired the mutex twice in the main thread (once in the CreateMutex call and a second time with WaitForSingleObject).

When you're finished writing to the file, you then release the mutex with this call:

if(!ReleaseMutex(m_hPortMutex))

But that only releases it once, so it's still owned by the main thread and the read thread will continue to block.

The bottom line is that you should set the bInitialOwner parameter to FALSE when creating the mutex.

m_hPortMutex = CreateMutex(NULL,FALSE,L"MY_MUTEX");

Quoting the CreateMutex documentation:

The thread that owns a mutex can specify the same mutex in repeated wait function calls without blocking its execution. [...] However, to release its ownership, the thread must call ReleaseMutex once for each time that the mutex satisfied a wait.

Answer 3

In your code, I see the following problems:

• The mutex is initially owned by the thread that called the constructor.
• Write() doesn't release the mutex on error.

I'm guessing that you call Write() from the same thread as the constructor call, which works because it already owns the mutex. After this, the mutex is still owned by the thread. making the other thread block.

I'd recommend that you move the release call outside of the check on whether the write suceeded (as you always need to release it).

You should note that you need to call ReleaseMutex() once for each time you acquire the mutex, by CreateMutex() with bInitialOwner set to TRUE, or by a successful WaitForSingleObject() call on the mutex.

Answer 4

Try creating the mutex with no name. Named mutexes are normally used for inter-process comms. Typically, a mutex used for comms between threads within a process are not named - they do not need to be opened by name because the mutex handle is acessible by all the process threads.