1

Closed

Bytes of data are sometimes left unread in the input queue

description

One of the workarounds for the Prolific PL2303 chipset involves ignoring serial port receive events during pending read requests. When it begins listening again for those events, it may have already missed some. I submitted a patch that adds checks for queued data and EOF characters that might have arrived while those events were ignored and mimics the actions taken by the normal event processing. See patch 17708.

file attachments

Closed Aug 1, 2015 at 3:34 PM by jmcurl

comments

perturbare wrote Jul 23, 2015 at 11:36 PM

Correction: Please see patch 17708.

perturbare wrote Jul 23, 2015 at 11:36 PM

Edited description to reflect more recent patch.

jmcurl wrote Jul 25, 2015 at 11:19 AM

After applying the patch, three test cases continuously fail on my machine.
  • SerialPortStream_ReadTo_Cached
  • SerialPortStream_ReadTo_Overflow
  • SerialPortStream_WriteReadline_Multiline.
The driver used for the tests is com0com that are set up as two end points. I'll look into more detail on what you're trying to do. So for now, I can't apply the patch.

jmcurl wrote Jul 25, 2015 at 12:17 PM

Keeping your code and removing "maskInUse", so it's the same logic as before at least lets the test cases pass.

What driver are you using that shows a problem?

After setting SetCommMask(maskReadPending) we don't wait for RX_CHAR, but do a read, and if data were to be placed after that read, setting the comm mask back with SetCommMask(maskRead) I would have expected the driver to have cached the result. That means if the SerialPortStream isn't working, the driver isn't doing this.

So I think the correct code is:
                    if (readPending) {
                        UnsafeNativeMethods.SetCommMask(m_ComPortHandle, maskReadPending);
                    } else {
                        UnsafeNativeMethods.SetCommMask(m_ComPortHandle, maskRead);

                        // While the comm event mask was set to ignore read events, data could have been written
                        // to the input queue. Check for that and if there are bytes waiting or EOF was received,
                        // set the appropriate flags.
                        uint bytesInQueue;
                        bool eofReceived;
                        if (GetReceiveStats(out bytesInQueue, out eofReceived) && (bytesInQueue > 0 || eofReceived)) {
                            // Tell DoReadEvent that there is data pending
                            m_ReadByteAvailable = true;
                            if (eofReceived) m_ReadByteEof |= EofByte.InDriver;
                        }
                    }
I've submitted that, please check r32876 if it works for you.

wrote Jul 25, 2015 at 8:45 PM

perturbare wrote Jul 28, 2015 at 4:11 PM

Myself and some of my coworkers were able to reproduce this 'unread byte' problem using three different USB-serial cables, plus one RS232 port on a laptop. We were connecting to a variety of Cisco devices.

For running the library tests, I have the following setup with two USB-serial converters connected by null modem cable:
  • Source: Prolific PL-2303 XA/HXA chip in a Gigaware cable. Gigaware driver v3.4.50.276.
  • Destination: FTDI ST232 variant with VID 403 and PID 6001 in an unlabeled cable. Driver version 2.12.4.0.
I've had problems when closing and reopening the ports quickly, at least with the Prolific. I get some test failures when I run them in batches and chalked it up to that problem. Most run just fine and the ones that fail in batch succeed if I run them again individually.

When SetCommMask is called to set back to maskRead, I don't think we get any cached read events. It looked to me as if every call to SetCommMask results in WaitCommEvent returning with lpEvtMask set to zero, which is consistent with Microsoft's documentation. It doesn't appear as if the events are retained internally and provided after the next call to SetCommMask.

I didn't try com0com. I'll have to give that a shot and see if I can find why my full set of changes didn't work for you.

Thanks.

Jon

perturbare wrote Jul 28, 2015 at 4:26 PM

I want to clarify that when I said, "Myself and some of my coworkers were able to reproduce this 'unread byte' problem," I was referring to our experience before making any changes. With the changes you accepted, that problem appears to be gone.

Those additional modifications that reduced the frequency of calls to SetCommMask were more for the sake of efficiency.

Thanks.

Jon

perturbare wrote Jul 28, 2015 at 5:47 PM

I did some testing using com0com-2.2.2.0-x64-fre-signed, using a release build of SerialPortStream.

Using SerialPortStream.NativeSerialPort.CommOverlappedIo.cs revision 32875
  • With ports set to emulate baud rate, enable buffer overrun, no test failures
  • With ports set NOT to emulate baud rate, enable buffer overrun, failures in SerialPortStream_SendReceive
Using SerialPortStream.NativeSerialPort.CommOverlappedIo.cs revision 32876
  • Same results
Using SerialPortStream.NativeSerialPort.CommOverlappedIo.cs with my full set of proposed changes
  • Same results
What version and what settings did you use?

jmcurl wrote Jul 28, 2015 at 8:29 PM

I'm using com0com version:
Driver Provider: Vyacheslav Frolov
Driver Date: 04/06/2012
Driver Version: 3.0.0.0
Digital Signer: Christos Nikolaou

I'm using the default bus for serial port pair emulator 0 (CNCA0 <-> CNCB0).

In the settings, I used for both ports as a virtual pair:
  • emubr=NO
  • emuoverrun=NO
  • pluginmode=NO
  • exclusivemode=NO
  • hiddenmode=NO
  • cts=RRTS
  • dsr=RDTR
  • dcd=RDTR
  • ri=!ON
  • emunoise=0
  • addrtto=0
  • addrito=0
Glad that I could apply your patch and it fixed your problem.

perturbare wrote Jul 29, 2015 at 5:17 PM

I decided to have a look at Microsoft's sample serial driver code to see if it provides any more detailed information on how the event mask is managed. If other driver implementations follow the same logic, each event is checked against the active mask before setting the bit. When maskReadPending is in effect, the receive events are not even recorded internally.

The sample I looked at is - https://code.msdn.microsoft.com/Serial-73a4564d

serial.h contains the following
    //
    // This mask will hold the bitmask sent down via the set mask
    // ioctl.  It is used by the interrupt service routine to determine
    // if the occurence of "events" (in the serial drivers understanding
    // of the concept of an event) should be noted.
    //
    ULONG IsrWaitMask;

    //
    // This mask will always be a subset of the IsrWaitMask.  While
    // at device level, if an event occurs that is "marked" as interesting
    // in the IsrWaitMask, the driver will turn on that bit in this
    // history mask.  The driver will then look to see if there is a
    // request waiting for an event to occur.  If there is one, it
    // will copy the value of the history mask into the wait request, zero
    // the history mask, and complete the wait request.  If there is no
    // waiting request, the driver will be satisfied with just recording
    // that the event occured.  If a wait request should be queued,
    // the driver will look to see if the history mask is non-zero.  If
    // it is non-zero, the driver will copy the history mask into the
    // request, zero the history mask, and then complete the request.
    //
    ULONG HistoryMask;
I looked through the places where those two variables are used and the behavior looks consistent with the comments. I hope this is useful information. I think it explains the behavior I was seeing and why the buffer check is required after setting the mask back to maskRead.

Thanks.

Jon

wrote Aug 1, 2015 at 3:34 PM

wrote Aug 1, 2015 at 3:34 PM