public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: cygwin-patches@cygwin.com
Subject: Re: Fix nanosleep returning negative rem
Date: Wed, 21 Jul 2021 11:30:50 +0200	[thread overview]
Message-ID: <YPfpSgbZbr+bnOWE@calimero.vinschen.de> (raw)
In-Reply-To: <0189b5495b2149c5a690de0431b7695c@metastack.com>

On Jul 21 09:07, David Allsopp wrote:
> > On Jul 20 16:16, David Allsopp wrote:
> > > I've pushed a repro case for this to
> > > https://github.com/dra27/cygwin-nanosleep-bug.git
> > >
> > > Originally noticed as the main CI system for OCaml has been failing
> > > sporadically for the signal.ml test mentioned in that repo. This
> > > morning I tried hammering that test on my dev machine and discovered
> > > that it fails very frequently. No idea if that's drivers, Windows 10
> > > updates, number of cores or what, but it was definitely happening, and
> > > easily.
> > >
> > > Drilling further, it appears that NtQueryTimer is able to return a
> > > negative value in the TimeRemaining field even when SignalState is
> > > false. The values I've seen have always been < 15ms - i.e. less than
> > > the timer resolution, so I wonder if there is a point at which the
> > > timer has elapsed but has not been signalled, but WaitForMultipleObjects
> > returns because of the EINTR signal.
> > > Mildly surprising that it seems to be so reproducible.
> > >
> > > Anyway, a patch is attached which simply guards a negative return
> > > value. The test on tbi.SignalState is in theory unnecessary.
> > 
> > Thanks for the patch, I think your patch is fine.  However, I'd like to
> > dig a bit into this to see what exactly happens.  Do you have a very
> > simple testcase in plain C, by any chance?
> 
> https://github.com/dra27/cygwin-nanosleep-bug/blob/main/signal.c was
> as simple as I'd gone at this stage (eliminating OCaml from the
> equation!). It might be possible to get it to happen without all the
> pthreads stuff: having confirmed it definitely wasn't OCaml and been
> able to put the appropriate system_printf's into cygwait to see that
> NtQueryTimer really was returning this small negative value, I stopped
> simplifying.
> 
> Does that repro case trigger on your system too?

I'm not sure.  Would the output " - nanosleep failed: ..." indicate the
bug has been triggered?  If so, no, I can't reproduce this on my system.

I wrote a quick STC using the NT API calls and I can't reproduce the
problem with this code either.  The output is either

  SignalState: 1 TimeRemaining: -5354077459183

or

  SignalState: 0 TimeRemaining: 653

I never get a small negative value in the latter case.  Can you
reproduce your problem with this testcase or tweak it to reproduce it?

Either way, your patch as safe guard should be ok.


Thanks,
Corinna

  reply	other threads:[~2021-07-21  9:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-20 15:16 David Allsopp
2021-07-21  8:39 ` Corinna Vinschen
2021-07-21  9:07   ` David Allsopp
2021-07-21  9:30     ` Corinna Vinschen [this message]
2021-07-21  9:33       ` Corinna Vinschen
2021-07-21 16:02         ` David Allsopp
2021-07-21 17:57           ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
2021-07-22  8:01           ` Corinna Vinschen

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=YPfpSgbZbr+bnOWE@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=cygwin-patches@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).