[newlib-cygwin] Cygwin: clocks: fix a hang on pre-Windows 10 machines

Corinna Vinschen corinna@sourceware.org
Sat Dec 1 14:15:00 GMT 2018


https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=2b72887ac834b0f0f675baff1af90771c7e36c87

commit 2b72887ac834b0f0f675baff1af90771c7e36c87
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Fri Nov 30 22:39:57 2018 +0100

    Cygwin: clocks: fix a hang on pre-Windows 10 machines
    
    when calling clocks too early in DLL init, the vtables are not correctly
    set up for some reason.  Calls to init() from now() fail because the init
    pointer in the vtable is NULL.
    
    Real life example is mintty which runs into a minor problem at startup,
    triggering a system_printf call.  Strace is another problem, it's called
    the first time prior to any class initialization.
    
    Workaround is to make sure that no virtual methods are called in an
    early stage.  Make init() non-virtual and convert resolution() to a
    virtual method instead.  Add a special non-virtual
    clk_monotonic_t::strace_usecs.
    
    While at it:
    
    - Inline internal-only methods.
    
    - Drop the `inited' member.  Convert period/ticks_per_sec toa union.
      Initialize period/ticks_per_sec via InterlockeExchange64.
    
    - Fix GetTickCount64 usage.  No, it's not returning ticks but
      milliseconds since boot (unbiased).
    
    - Fix comment indentation.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/clock.cc  | 73 +++++++++++++++++++++++++++----------------------
 winsup/cygwin/clock.h   | 40 +++++++++++++++++++--------
 winsup/cygwin/strace.cc |  8 ++----
 3 files changed, 71 insertions(+), 50 deletions(-)

diff --git a/winsup/cygwin/clock.cc b/winsup/cygwin/clock.cc
index 0c8390a..18d6288 100644
--- a/winsup/cygwin/clock.cc
+++ b/winsup/cygwin/clock.cc
@@ -5,7 +5,7 @@
 #include "miscfuncs.h"
 #include "spinlock.h"
 
-static LONGLONG
+static inline LONGLONG
 system_qpc_tickspersec ()
 {
   LARGE_INTEGER qpf;
@@ -15,7 +15,7 @@ system_qpc_tickspersec ()
   return qpf.QuadPart;
 }
 
-static LONGLONG
+static inline LONGLONG
 system_tickcount_period ()
 {
   ULONG coarsest = 0, finest, actual;
@@ -23,40 +23,37 @@ system_tickcount_period ()
   if (!coarsest)
     {
       /* The actual resolution of the OS timer is a system-wide setting which
-       can be changed any time, by any process.  The only fixed value we
-       can rely on is the coarsest value. */
+	 can be changed any time, by any process.  The only fixed value we
+	 can rely on is the coarsest value. */
       NtQueryTimerResolution (&coarsest, &finest, &actual);
     }
   return coarsest;
 }
 
-void
+void inline
 clk_t::init ()
 {
-  spinlock spin (inited, 1);
-  if (!spin)
-    period = system_tickcount_period ();
+  if (!period)
+    InterlockedExchange64 (&period, system_tickcount_period ());
 }
 
-void
+void inline
 clk_realtime_t::init ()
 {
-  spinlock spin (inited, 1);
-  if (!spin)
+  if (wincap.has_precise_system_time ())
     {
-      if (wincap.has_precise_system_time ())
-	ticks_per_sec = system_qpc_tickspersec ();
-      else
-	period = system_tickcount_period ();
+      if (!ticks_per_sec)
+	InterlockedExchange64 (&ticks_per_sec, system_qpc_tickspersec ());
     }
+  else if (!period)
+    InterlockedExchange64 (&period, system_tickcount_period ());
 }
 
-void
+void inline
 clk_monotonic_t::init ()
 {
-  spinlock spin (inited, 1);
-  if (!spin)
-    ticks_per_sec = system_qpc_tickspersec ();
+  if (!ticks_per_sec)
+    InterlockedExchange64 (&ticks_per_sec, system_qpc_tickspersec ());
 }
 
 int
@@ -169,8 +166,7 @@ clk_monotonic_t::now (clockid_t clockid, struct timespec *ts)
       LARGE_INTEGER now;
       struct timespec bts;
 
-      if (inited <= 0)
-	init ();
+      init ();
       do
 	{
 	  bias = SharedUserData.InterruptTimeBias;
@@ -209,13 +205,10 @@ clk_monotonic_coarse_t::now (clockid_t clockid, struct timespec *ts)
       /* Vista-only: GetTickCount64 is biased but it's coarse and monotonic. */
       ULONGLONG now;
 
-      if (inited <= 0)
-	init ();
-      now = GetTickCount64 ();
-      now *= period;	/* Normalize to 100ns period */
-      ts->tv_sec = now / NS100PERSEC;
-      now %= NS100PERSEC;
-      ts->tv_nsec = now * (NSPERSEC/NS100PERSEC);
+      now = GetTickCount64 ();	/* Returns ms since boot */
+      ts->tv_sec = now / MSPERSEC;
+      now %= MSPERSEC;
+      ts->tv_nsec = now * (NSPERSEC/MSPERSEC);
     }
   return 0;
 }
@@ -237,8 +230,7 @@ clk_boottime_t::now (clockid_t clockid, struct timespec *ts)
     {
       LARGE_INTEGER now;
 
-      if (inited <= 0)
-	init ();
+      init ();
       QueryPerformanceCounter (&now);
       ts->tv_sec = now.QuadPart / ticks_per_sec;
       now.QuadPart %= ticks_per_sec;
@@ -250,15 +242,30 @@ clk_boottime_t::now (clockid_t clockid, struct timespec *ts)
 void
 clk_t::resolution (struct timespec *ts)
 {
-  if (inited <= 0)
-    init ();
+  init ();
+  ts->tv_sec = 0;
+  ts->tv_nsec = period * (NSPERSEC/NS100PERSEC);
+}
+
+void
+clk_realtime_t::resolution (struct timespec *ts)
+{
+  init ();
   ts->tv_sec = 0;
-  if (ticks_per_sec)
+  if (wincap.has_precise_system_time ())
     ts->tv_nsec = NSPERSEC / ticks_per_sec;
   else
     ts->tv_nsec = period * (NSPERSEC/NS100PERSEC);
 }
 
+void
+clk_monotonic_t::resolution (struct timespec *ts)
+{
+  init ();
+  ts->tv_sec = 0;
+  ts->tv_nsec = NSPERSEC / ticks_per_sec;
+}
+
 static clk_realtime_coarse_t clk_realtime_coarse;
 static clk_realtime_t clk_realtime;
 static clk_process_t clk_process;
diff --git a/winsup/cygwin/clock.h b/winsup/cygwin/clock.h
index 075aaed..c05bf47 100644
--- a/winsup/cygwin/clock.h
+++ b/winsup/cygwin/clock.h
@@ -52,13 +52,15 @@ details. */
 class clk_t
 {
  protected:
-  LONG inited;
   /* Some values are returned as ticks/s, some as 100ns period of a
      single tick.  Store the original value and use a computation method
      making the most sense for the value given, to avoid rounding issues. */
-  LONGLONG ticks_per_sec;
-  LONGLONG period;
-  virtual void init ();
+  union
+    {
+      LONGLONG ticks_per_sec;
+      LONGLONG period;
+    };
+  void init ();
   virtual int now (clockid_t, struct timespec *) = 0;
 
  public:
@@ -66,32 +68,35 @@ class clk_t
   {
     return now (_id, ts);
   }
-  void resolution (struct timespec *);
+  virtual void resolution (struct timespec *);
 
   /* shortcuts for non-process/thread clocks */
-  void nsecs (struct timespec *ts) { nsecs (0, ts); }
+  void nsecs (struct timespec *ts)
+  {
+    now (0, ts);
+  }
   ULONGLONG nsecs ()
   {
     struct timespec ts;
-    nsecs (&ts);
+    now (0, &ts);
     return (ULONGLONG) ts.tv_sec * NSPERSEC + ts.tv_nsec;
   }
   LONGLONG n100secs ()
   {
     struct timespec ts;
-    nsecs (&ts);
+    now (0, &ts);
     return ts.tv_sec * NS100PERSEC + ts.tv_nsec / (NSPERSEC/NS100PERSEC);
   }
   LONGLONG usecs ()
   {
     struct timespec ts;
-    nsecs (&ts);
+    now (0, &ts);
     return ts.tv_sec * USPERSEC + ts.tv_nsec / (NSPERSEC/USPERSEC);
   }
   LONGLONG msecs ()
   {
     struct timespec ts;
-    nsecs (&ts);
+    now (0, &ts);
     return ts.tv_sec * MSPERSEC + ts.tv_nsec / (NSPERSEC/MSPERSEC);
   }
 };
@@ -103,8 +108,10 @@ class clk_realtime_coarse_t : public clk_t
 
 class clk_realtime_t : public clk_t
 {
-  virtual void init ();
+  void init ();
   virtual int now (clockid_t, struct timespec *);
+ public:
+  virtual void resolution (struct timespec *);
 };
 
 class clk_process_t : public clk_t
@@ -120,9 +127,18 @@ class clk_thread_t : public clk_t
 class clk_monotonic_t : public clk_t
 {
  protected:
-  virtual void init ();
+  void init ();
  private:
   virtual int now (clockid_t, struct timespec *);
+ public:
+  virtual void resolution (struct timespec *);
+  /* Under strace 1st call is so early that vtable is NULL. */
+  LONGLONG strace_usecs ()
+  {
+    struct timespec ts;
+    clk_monotonic_t::now (0, &ts);
+    return ts.tv_sec * USPERSEC + ts.tv_nsec / (NSPERSEC/USPERSEC);
+  }
 };
 
 class clk_monotonic_coarse_t : public clk_t
diff --git a/winsup/cygwin/strace.cc b/winsup/cygwin/strace.cc
index 21c2d6d..35f8a59 100644
--- a/winsup/cygwin/strace.cc
+++ b/winsup/cygwin/strace.cc
@@ -82,14 +82,12 @@ strace::dll_info ()
 int
 strace::microseconds ()
 {
-  /* Need a local clock instance because this function is called before
-     the global constructors of the inferior process have been called. */
-  static clk_monotonic_t clock_monotonic;
   static LONGLONG process_start NO_COPY;
+  clk_monotonic_t *clk = (clk_monotonic_t *) get_clock (CLOCK_MONOTONIC);
 
   if (!process_start)
-    process_start = clock_monotonic.usecs ();
-  return (int) (clock_monotonic.usecs () - process_start);
+    process_start = clk->strace_usecs ();
+  return (int) (clk->strace_usecs () - process_start);
 }
 
 static int __stdcall



More information about the Cygwin-cvs mailing list