Reputation: 37
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
Reputation: 4350
It is bad logic as a whole. For example, there are the following issues.
You may want to redesign your program with reference to the following articles and source code:
Synchronization and Overlapped Input and Output
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
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