Radek Hlavinka
Radek Hlavinka

Reputation: 37

WaitForSingleObject() gets signalled without occurrence of awaited event

I'm writing a Class for COM ports in C++ using win-api. Righ now I test the functionality on RS232 with connected Rx and Tx pins.

I've encountered somewhat weird problem. I use separate thread for reading from the COM port. Within the thread I use SetCommMask, WaitCommEvent and WaitForSingleObject to wait for char arrival into buffer. However the WaitForSingleObject tends to exit without actually receiving any chars.

I assumed this was caused by wrong use of mentioned functions, but then I discovered, that premature exit does not occur every time (first time always works as intended).

In second go the thread enters the waiting state and exits a while later proceeding to ReadFile, where it waits indefinitely, because the buffer is empty, no data is to be sent and Total timeout is not used.

I've already been advised to simply use ReadFile and process only the data I acquire, however I use another thread to check if the communication channel has been disconnected and right now I need to distinguish between waiting for data and reading data.

Calling ClearCommError to check input buffer with ReadFile is not an option, because in such a case InQue is always 0. Therefore I cannot tell whether ReadFile is actually reading or waiting.

    //following code runs in separate thread
    DWORD dwEventMask1, dwEventMask2, LastError, Status;
    OVERLAPPED Overlapped; HANDLE Serial_Port_Handle;
    std::string stringBuffer("");
    const size_t ReadBufferLength = 256;
    char tempBuffer[ReadBufferLength];

    GetCommMask(Serial_Port_Handle, &dwEventMask1);



    if (dwEventMask1)   // Before starting the thread I check the state of Input Buffer with GetCommError().
    {                   // If Buffer is not empty, CommMask is set to 0 signaling there is no need for waiting.
        Overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

        //wait for comm event
        if (!WaitCommEvent(Serial_Port_Handle, &dwEventMask1, &Overlapped))
        {
            if ((LastError = GetLastError()) == ERROR_IO_PENDING)
            {
                Waiting = true; //signal bool for synchronization purposes
                if ((Status = WaitForSingleObject(Overlapped.hEvent, INFINITE)) == WAIT_OBJECT_0)
                {
                    GetCommMask(Serial_Port_Handle, &dwEventMask2);
                    Waiting = false;
                    CloseHandle(Overlapped.hEvent); 
                    // I close handle and set all members of Overlapped struct to 0
                } 

                if (dwEventMask2 !== dwEventMask1) // check if wait have not exited because of SetCommMast()
                    return;
            }
        }
    }
    do  // this loop reads from input buffer until empty
    {
        Overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);//set up read overlapped operation

        if (ReadFile(Serial_Port_Handle, tempBuffer, ReadBufferLength - 1, &NoBytesRead, &Overlapped)) //start read opperation
        { //Read operation done on 1 go
            GetOverlappedResult(Serial_Port_Handle, &Overlapped, &NoBytesRead, FALSE); //get NoBytesRead

            CloseHandle(Overlapped.hEvent)
            stringBuffer.append(tempBuffer, (size_t) NoBytesRead); // save read data
        }
        else if ((LastError = GetLastError()) == ERROR_IO_PENDING) //operation not yet complete
        {
            GetOverlappedResult(Serial_Port_Handle, &Overlapped, &NoBytesRead, TRUE); // wait for completion
            stringBuffer.append(tempBuffer, (size_t)NoBytesRead);
        }
        else
        {
            CloseHandle(Overlapped.hEvent)
            return;
        }
    } while ((NoBytesRead == (ReadBufferLength - 1)) && NoBytesRead);
    // Loop runs while tempBuffer's full capacity is used.
    // I realize that since I don't use Total Timeout there is a possibility
    // of the loop getting stuck. If you can suggest other solution than using
    // Total Timeout or use GetCommError() to get input buffer state, please do so.
    return;

This code is somewhat simplified (checking for return values, ets.).

1) Have any of you experienced such a behaviour?

2) I use OVERLAPPED operations in the code. After the operation exits I always use CloseHandle and reinitialize the OVERLAPPED structure before using it for another operation. Is this correct or is resetting the structure sufficient?

Upvotes: 1

Views: 2647

Answers (2)

kunif
kunif

Reputation: 4350

It is bad logic as a whole. For example, there are the following issues.

  • CreateEvent/CloseHandle should not executed at every ReadFile/WaitCommEvent.
  • The usage of GetCommMask/WaitCommEvent is also wrong.
  • The read data size specified in ReadFile is fixed regardless of the situation.
  • It also includes the comment on @RbMm.

You may want to redesign your program with reference to the following articles and source code:

Synchronization and Overlapped Input and Output

Serial Communications

bmo/mttty


In Addition:
I did not notice that a file handle (not an event handle) was specified for WaitForSingleObject, as pointed out by @Rita Han.
The biggest problem is that.

However, the situation that it is better to redesign has not changed.
There is no description of WaitCommEvent and Overlapped for it in the source of @Rita Han's answer. Also, the read data size is fixed in ReadFile.


On the other hand:
Although it does not occur in the source code of the question, it is possible for WaitCommEvent/WaitForSingleObject to generate an event that is not specified in SetCommMask.

  • While the WaitCommEvent is waiting for completion, change the event mask with SetCommMask.
    Remarks - WaitCommEvent function

    If a process attempts to change the device handle's event mask by using the SetCommMask function while an overlapped WaitCommEvent operation is in progress, WaitCommEvent returns immediately. The variable pointed to by the lpEvtMask parameter is set to zero.

  • While WaitCommEvent is waiting for completion, call WaitCommEvent multiple times using the same Overlapped structure.
    Synchronization and Overlapped Input and Output

    When performing multiple simultaneous overlapped operations on a single thread, the calling thread must specify an OVERLAPPED structure for each operation. Each OVERLAPPED structure must specify a handle to a different manual-reset event object.

    A thread should not reuse an event with the assumption that the event will be signaled only by that thread's overlapped operation. An event is signaled on the same thread as the overlapped operation that is completing. Using the same event on multiple threads can lead to a race condition in which the event is signaled correctly for the thread whose operation completes first and prematurely for other threads using that event.

The document is described as above, but depending on the device driver/vendor, the WaitCommEvent that is called later ends with an error, and the WaitCommEvent waiting for completion is lpEvtMask return with zero (as in SetCommMask).


For multiple Overlapped structure variables:
A common programming know-how is that using a single variable for multiple purposes is prone to bugs.
If you are designing in asynchronous and/or multi-threading, it is better to prepare at least three Overlapped structure variables for ReadFile, WriteFile, WaitCommEvent.

About starting ReadFile regardless of the state of the input buffer:
This is about calling ReadFile with a fixed length of 256 bytes without acquiring the size of the received data in the input buffer of the device driver.

In fact, even if all the data arrives, if it is less than 256 bytes, it will always be delayed until a 256 byte receive timeout occurs.

For example, the loop reads one byte at a time until a timeout error occurs, which means the end of the received data (1-byte read timeout would have no impact).
Or, as answered in the previous article, use ClearCommError to obtain the size of the data stored in the device driver's input buffer, and call ReadFile specifying that size.

There will be no problem with the application-side buffer handling you are explaining.

About the behavior of WaiCommEvent when calling SetCommMask:
It may depend on the device driver you are using.

Upvotes: 1

Rita Han
Rita Han

Reputation: 9700

However the WaitForSingleObject tends to exit without actually receiving any chars.

Wait for the event handle instead of serial device handle.

I got it working. The following is my code example you can have a try:

DWORD errCode = 0;
BOOL result = false;

HANDLE serialDeviceHdl = CreateFile(L"COM8", GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
if (!serialDeviceHdl)
{
    errCode = GetLastError();
    cout << "Open device failed. Error code: " << errCode << endl;
    return 0;
}

OVERLAPPED overlappedForWrite = {};
overlappedForWrite.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

DWORD writenSize = 0;
result = WriteFile(serialDeviceHdl, "hello", 5, &writenSize, &overlappedForWrite);
if (FALSE == result)
{
    errCode = GetLastError();
    if (ERROR_IO_PENDING == errCode)
    {
        cout << "Overlapped I/O operation is in progress." << endl;
    }
    else
    {
        cout << "Write to device failed. Error code: " << errCode << endl;
    }
}

DWORD returnValue = WaitForSingleObject(overlappedForWrite.hEvent, INFINITE);
if (WAIT_OBJECT_0 == returnValue)
{
    cout << "The state of the specified object is signaled." << endl;
}
else
{
    cout << "Wait for single object failed. Error code: " << returnValue << endl;
}

CHAR readBuf[5];
DWORD readSize = 0;
OVERLAPPED overlappedForRead = {};
overlappedForRead.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

result = ReadFile(serialDeviceHdl, readBuf, 5, &readSize, &overlappedForRead);
if (FALSE == result)
{
    errCode = GetLastError();
    if (ERROR_IO_PENDING == errCode)
    {
        cout << "Overlapped I/O operation is in progress." << endl;
    }
    else
    {
        cout << "Write to device failed. Error code: " << errCode << endl;
    }
}

returnValue = WaitForSingleObject(overlappedForRead.hEvent, INFINITE);
if (WAIT_OBJECT_0 == returnValue)
{
    cout << "The state of the specified object is signaled." << endl;
}
else
{
    cout << "Wait for single object failed. Error code: " << returnValue << endl;
}

Upvotes: 0

Related Questions