[Bug 10056] New: False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Summary: False error on serial port read
           Product: Wine
           Version: CVS/GIT
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: wine-kernel
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=8604)
 --> (http://bugs.winehq.org/attachment.cgi?id=8604)
An internal hack used to work around the problem.

We use an application which reads from a serial port or emulated serial port.
This stopped working with recent versions of Wine, and bisection tracked it
down to this commit:

http://source.winehq.org/git/wine.git/?a=commitdiff;h=539d5863e6fae78943fe59...

This commit makes NtReadFile() return STATUS_PIPE_BROKEN whenever the read()
system call returns 0. Unfortunately, Wine sets VMIN to 0, forcing read() to
return 0 whenever there is no data available:

http://source.winehq.org/source/dlls/ntdll/serial.c#L752

Our application sees STATUS_PIPE_BROKEN, and immediately fails.

I've attached a patch which we use to work around this problem, but I'd like to
work towards a more elegant solution.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

--- Comment #1 from Alexandre Julliard   2007-10-16 09:27:02 ---
The right way would be to have read() return EAGAIN in that case, so that we
can do the async I/O or timeout handling manually. You could try to set VMIN to
1 and VTIME to 0, though I'm not sure this would do the right thing on all
platforms.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Adrian Cox  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #8604 is|0                           |1
           obsolete|                            |

--- Comment #2 from Adrian Cox   2007-10-16 10:48:41 ---
Created an attachment (id=8611)
 --> (http://bugs.winehq.org/attachment.cgi?id=8611)
Cleaner Patch

The attached is a cleaner patch, which forces the code down the EAGAIN path,
but only for files of type FD_TYPE_SERIAL. This fixes the bug for me, and
should have a narrower impact than the previous patch.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|kernel32                    |ntdll

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Wolfgang Walter  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wolfgang.walter@studentenwer
                   |                            |k.mhn.de

--- Comment #3 from Wolfgang Walter   2008-01-07 13:35:19 ---
Posix does not define whether VMIN/VTIME mechanism or O_NONBLOCK has preference
in "Non-canonical Mode Input Processing":

See

http://www.opengroup.org/onlinepubs/007908799/xbd/termios.html

"In non-canonical mode input processing, input bytes are not assembled into
lines, and erase and kill processing does not occur. The values of the MIN and
TIME members of the c_cc array are used to determine how to process the bytes
received. The ISO POSIX-1 standard does not specify whether the setting of
O_NONBLOCK takes precedence over MIN or TIME settings. Therefor, if O_NONBLOCK
is set, read() may return immediately, regardless of the setting of MIN or
TIME. Also, if no data is available, read() may either return 0, or return -1
with errno set to [EAGAIN]."

I think that i.e. Linux returns with 0 instead of -1, errno=EAGAIN in this case
whereas Solarix always returns with -1, errno=EAGAIN.

Setting VMIN=1 probably does not solve the problem as then read may block on
some OS even for non-blocking handles. (Though for linux it should work).

The last patch of Adrian Cox isn't a perfect solution, either. With VMIN=0 EOF
is not detected any more for serial devices. This is a regression for systems
where O_NONBLOCK has priority over VMIN/VTIME (On the other hand this patch
makes serial devices work at all on those systems where VMIN,VTIME semantics
win).

Another workaround is to check for serial fds if there is actually something to
read:

for (;;)
{
    int n = 0;
    if (type != FD_TYPE_SERIAL || (ioctl(unix_handle, FIONREAD, &n) != -1 &&
n>0))
    {
        if ((result = read( unix_handle, (char *)buffer + total, length - total
)) >= 0)
        {
            total += result;
            .....
        }
        else
        {
            .....
        }
    }
    if (!(options & (FILE_SYNCHRONOUS_IO_ALERT |
FILE_SYNCHRONOUS_IO_NONALERT)))
.....
.....
}

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Adrian Cox  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #8611 is|0                           |1
           obsolete|                            |

--- Comment #4 from Adrian Cox   2008-03-28 11:23:39 ---
Created an attachment (id=11714)
 --> (http://bugs.winehq.org/attachment.cgi?id=11714)
Don't attempt initial read for serial ports

This patch is based on Wolfgang Walter's comment. The aim is to revert serial
ports to the original Wine behaviour, and to queue the asynchronous I/O without
attempting a read first. The approach should be independent of the operating
system's interpretation of Posix serial port behaviour: as explained in the
comment above, the current approach is vulnerable to an ambiguity in Posix.

The patch is very simple, and should not affect anything other than serial
ports, but it does modify a larger number of lines in order to preserve the
file indentation.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

--- Comment #5 from Adrian Cox   2008-04-11 05:45:39 ---
Created an attachment (id=12063)
 --> (http://bugs.winehq.org/attachment.cgi?id=12063)
Test case that demonstrates difference between Wine and Windows

This small program is a test case that demonstrates the behaviour difference
between Wine and Windows. On Windows the serial timeout is honoured, and the
ReadFile call returns success, with zero bytes read, once the timeout expires.

On unpatched wine, the ReadFile call returns immediately with
ERROR_BROKEN_PIPE.

Wine patched with my suggested fix behaves as Windows.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch, testcase

--- Comment #6 from Austin English   2008-04-11 12:23:43 ---
Can you add that testcase to the wine test suite?

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

--- Comment #7 from Adrian Cox   2008-04-11 13:15:22 ---
(In reply to comment #6)
> Can you add that testcase to the wine test suite?

The difficulty is that the test needs to know a serial port which exists, but
is not connected to any active hardware. The test will not work if there is a
GPS or similar device connected to the chosen port. I'd like any suggestions to
get around that problem.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #8 from Alexandre Julliard   2008-05-07 13:53:57 ---
This should be fixed now.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #9 from Alexandre Julliard   2008-05-09 12:54:45 ---
Closing bugs fixed in 1.0-rc1.

[Bug 10056] False error on serial port read

http://bugs.winehq.org/show_bug.cgi?id=10056

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|CVS/GIT                     |unspecified