public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: cygwin@cygwin.com
Subject: Re: Setting termios VMIN > 0 and VTIME > 0 on non blocking file
Date: Thu, 12 Mar 2020 15:44:45 +0100	[thread overview]
Message-ID: <20200312144445.GP4042@calimero.vinschen.de> (raw)
In-Reply-To: <365dd437-7553-eb4e-3253-aba3bab74895@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3010 bytes --]

On Mar 12 15:20, Åke Rehnman via Cygwin wrote:
> 
> On 2020-03-12 12:40, Corinna Vinschen wrote:
> > 
> > I inspected the serial I/O read function and I only see a subtil
> > difference in terms of VMIN/VTIME which doesn't seem to be the culprit
> > at first glance.  In O_NONBLOCK mode, the underlying Windows function
> > ReadFile is called unconditionally.  My current hunch is this:
> > 
> > - If VMIN>0 && VTIME>0, the Windows equivalent of tcsetattr is
> >    told that VMIN>0.
> > 
> > - So, assuming VMIN == 2 in Windows.  If VTIME > 0, the Cygwin read
> >    function sets the number of bytes_to_read to 1 (this is old code,
> >    don't ask why).
> Do you have a pointer to this? I can not find it...

That's because it's not true.  I created this mail during lunch break
and screwed up.

> > - My assumption now is that the ReadFile function fails because it's
> >    supposed to return only 1 char, but VMIN is > 1.
> > 
> > There are two issues here.
> > 
> > - If my assumption is correct, the tcsetattr function must not
> >    set the Windows VMIN equivalent to != 0 in the O_NONBLOCK case.
> > 
> > - Also, as mention above, if VTIME > 0, Cygwin's read sets the number of
> >    bytes_to_read to 1.   Weirdly, it does so even if there are more than
> >    1 byte in the inbound queue.  This sounds wrong to me.
> > 
> I think the problem is if the number of bytes requested are more than what

To clarify: number of bytes == VMIN?

> is in the buffer it is going to overlap the read function (because of VTIME)
> and immediately after that CancelIO is called. Contrary to what is mentioned
> in the source code I think CancelIO is actually discarding data...

So far we didn't have that experience.  CancelIO is usually safe
in this regard.

>   /* Use CancelIo rather than PurgeComm (PURGE_RXABORT) since
>          PurgeComm apparently discards in-flight bytes while CancelIo
>          only stops the overlapped IO routine. */
> 
> 
> My suggestion is the following patch:
> 
> diff --git a/winsup/cygwin/fhandler_serial.cc
> b/winsup/cygwin/fhandler_serial.cc
> index 69e5768f6..afa8871bf 100644
> --- a/winsup/cygwin/fhandler_serial.cc
> +++ b/winsup/cygwin/fhandler_serial.cc
> @@ -898,7 +898,11 @@ fhandler_serial::tcsetattr (int action, const struct
> termios *t)
>    {
>      memset (&to, 0, sizeof (to));
> 
> -    if ((vmin_ > 0) && (vtime_ == 0))
> +       if (is_nonblocking())
> +       {
> +               to.ReadIntervalTimeout = MAXDWORD;
> +       }
> +    else if ((vmin_ > 0) && (vtime_ == 0))

What if you switch to !O_NONBLOCK after calling tcsetattr?  The
setting of ReadIntervalTimeout would be lost then.

Either we have to repeat calling SetCommTimeouts every time
we switch mode, or we have to do the above setting temporary
every time we call ReadFile in non blocking mode.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-03-12 14:44 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <mailman.1473.1583981776.15804.cygwin@cygwin.com>
2020-03-11 20:48 ` Åke Rehnman
2020-03-11 21:55   ` Brian Inglis
2020-03-12  1:01     ` Norton Allen
2020-03-12  1:04     ` Åke Rehnman
2020-03-12  1:08       ` Norton Allen
2020-03-12  1:39         ` Åke Rehnman
2020-03-12  8:05   ` Thomas Dickey
2020-03-12 11:01     ` Åke Rehnman
2020-03-12 19:48     ` Achim Gratz
2020-03-12 11:40   ` Corinna Vinschen
2020-03-12 13:32     ` Åke Rehnman
2020-03-12 14:13       ` Corinna Vinschen
2020-03-12 16:42         ` Åke Rehnman
2020-03-13 10:02           ` Corinna Vinschen
2020-03-12 14:20     ` Åke Rehnman
2020-03-12 14:44       ` Corinna Vinschen [this message]
2020-03-12 15:08         ` Corinna Vinschen
2020-03-12 17:04           ` Åke Rehnman
2020-03-13 10:12             ` Corinna Vinschen
2020-03-14 10:23               ` Åke Rehnman
2020-03-14 10:36                 ` Åke Rehnman
2020-03-16  9:35                   ` Corinna Vinschen
2020-03-17 12:16                     ` Corinna Vinschen
2020-03-17 15:54                       ` Åke Rehnman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200312144445.GP4042@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=cygwin@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).