[newlib-cygwin] Cygwin: timerfd: implement fork semantics

Corinna Vinschen corinna@sourceware.org
Wed Jan 16 14:34:00 GMT 2019


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

commit 4195bae67f19b02e860678c8d2c2db3eddd4276a
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Wed Jan 16 15:33:15 2019 +0100

    Cygwin: timerfd: implement fork semantics
    
    - Puzzeling: Commit ec98d19a08c2e4678e8a6f40fea0c9bbeaa4a2c7
      changed ttstart to NO_COPY but kept all the code to handle
      fixup after fork.  Revert to not-NO_COPY and make timerfd
      fork work.
    
    - On fixup_after_fork, keep timerfd timers and restart thread
      if they were armed in the parent.
    
    - Move timerfd timer_trackers to cygheap.  Overload timer_tracker
      new and delete methods to handle timers accordingly.  This is not
      exactly required for fork, but exec will be grateful.
    
    - Give up on TFD_TIMER_CANCEL_ON_SET for now.  There's no easy way
      to recognize a discontinuous change in a clock.
    
    - Be paranoid when cleaning out ttstart.
    
    - Fix some minor issues.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/fhandler.h          |  4 +---
 winsup/cygwin/fhandler_timerfd.cc | 19 +++++++++------
 winsup/cygwin/timer.cc            | 49 +++++++++++++++++++++++++++++++--------
 winsup/cygwin/timer.h             |  6 +++++
 4 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 898f85d..f0c612d 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2699,9 +2699,7 @@ class fhandler_timerfd : public fhandler_base
 
   HANDLE get_timerfd_handle ();
 
-  void fixup_after_fork_exec (bool);
-  void fixup_after_exec () {fixup_after_fork_exec (true);}
-  void fixup_after_fork (HANDLE) {fixup_after_fork_exec (false);}
+  void fixup_after_exec ();
 
   select_record *select_read (select_stuff *);
   select_record *select_write (select_stuff *);
diff --git a/winsup/cygwin/fhandler_timerfd.cc b/winsup/cygwin/fhandler_timerfd.cc
index b88c797..8ce91af 100644
--- a/winsup/cygwin/fhandler_timerfd.cc
+++ b/winsup/cygwin/fhandler_timerfd.cc
@@ -27,10 +27,19 @@ fhandler_timerfd::get_proc_fd_name (char *buf)
   return strcpy (buf, "anon_inode:[timerfd]");
 }
 
+/* The timers connected to a descriptor are stored on the cygheap
+   together with their fhandler. */
+
+#define cnew(name, ...) \
+  ({ \
+    void* ptr = (void*) ccalloc (HEAP_FHANDLER, 1, sizeof (name)); \
+    ptr ? new (ptr) name (__VA_ARGS__) : NULL; \
+  })
+
 int
 fhandler_timerfd::timerfd (clockid_t clock_id, int flags)
 {
-  timerid = (timer_t) new timer_tracker (clock_id, NULL, true);
+  timerid = (timer_t) cnew (timer_tracker, clock_id, NULL, true);
   if (flags & TFD_NONBLOCK)
     set_nonblocking (true);
   if (flags & TFD_CLOEXEC)
@@ -150,13 +159,9 @@ fhandler_timerfd::dup (fhandler_base *child, int flags)
 }
 
 void
-fhandler_timerfd::fixup_after_fork_exec (bool execing)
+fhandler_timerfd::fixup_after_exec ()
 {
-  if (!execing)
-    {
-      /* TODO after fork */
-    }
-  else if (!close_on_exec ())
+  if (!close_on_exec ())
     {
       /* TODO after exec */
     }
diff --git a/winsup/cygwin/timer.cc b/winsup/cygwin/timer.cc
index c429af6..bd412f0 100644
--- a/winsup/cygwin/timer.cc
+++ b/winsup/cygwin/timer.cc
@@ -22,7 +22,8 @@ details. */
 #define EVENT_ARMED	-1
 #define EVENT_LOCK	 1
 
-timer_tracker NO_COPY ttstart (CLOCK_REALTIME, NULL, false);
+/* Must not be NO_COPY, otherwise timerfd breaks. */
+timer_tracker ttstart (CLOCK_REALTIME, NULL, false);
 
 class lock_timer_tracker
 {
@@ -79,7 +80,8 @@ timer_tracker::~timer_tracker ()
 /* fd is true for timerfd timers. */
 timer_tracker::timer_tracker (clockid_t c, const sigevent *e, bool fd)
 : magic (TT_MAGIC), instance_count (1), clock_id (c), deleting (false),
-  hcancel (NULL), syncthread (NULL), event_running (EVENT_DISARMED),
+  hcancel (NULL), syncthread (NULL), timerfd_event (NULL),
+  interval_us(0), sleepto_us(0), event_running (EVENT_DISARMED),
   overrun_count_curr (0), overrun_count (0)
 {
   if (e != NULL)
@@ -171,7 +173,7 @@ timer_tracker::disarm_event ()
 LONG64
 timer_tracker::wait (bool nonblocking)
 {
-  HANDLE w4[3] = { NULL, hcancel, timerfd_event };
+  HANDLE w4[3] = { NULL, hcancel, get_timerfd_handle () };
   LONG64 ret = -1;
 
   wait_signal_arrived here (w4[0]);
@@ -264,7 +266,11 @@ timer_tracker::thread_func ()
 	  {
 	    if (!timerfd_event)
 	      break;
-	    arm_event ();
+	    if (arm_event ())
+	      {
+		debug_printf ("%p timerfd already queued", this);
+		break;
+	      }
 	    SetEvent (timerfd_event);
 	    break;
 	  }
@@ -444,19 +450,40 @@ timer_tracker::close (timer_tracker *tt)
 }
 
 void
+timer_tracker::restart ()
+{
+  timerfd_event = CreateEvent (&sec_none_nih, TRUE, FALSE, NULL);
+  if (interval_us != 0 || sleepto_us != 0)
+    {
+      hcancel = CreateEvent (&sec_none_nih, TRUE, FALSE, NULL);
+      syncthread = CreateEvent (&sec_none_nih, TRUE, FALSE, NULL);
+      new cygthread (timer_thread, this, "itimer", syncthread);
+    }
+}
+
+void
 timer_tracker::fixup_after_fork ()
 {
-  /* TODO: Keep timerfd timers available and restart them */
   ttstart.hcancel = ttstart.syncthread = NULL;
+  ttstart.interval_us = ttstart.sleepto_us = 0;
   ttstart.event_running = EVENT_DISARMED;
-  ttstart.overrun_count_curr = 0;
-  ttstart.overrun_count = 0;
+  ttstart.overrun_count_curr = ttstart.overrun_count = 0;
   for (timer_tracker *tt = &ttstart; tt->next != NULL; /* nothing */)
     {
       timer_tracker *deleteme = tt->next;
-      tt->next = deleteme->next;
-      deleteme->hcancel = deleteme->syncthread = NULL;
-      delete deleteme;
+      if (deleteme->get_timerfd_handle ())
+	{
+	  tt = deleteme;
+	  tt->restart ();
+	}
+      else
+	{
+	  tt->next = deleteme->next;
+	  deleteme->timerfd_event = NULL;
+	  deleteme->hcancel = NULL;
+	  deleteme->syncthread = NULL;
+	  delete deleteme;
+	}
     }
 }
 
@@ -731,6 +758,8 @@ extern "C" int
 timerfd_settime (int fd_in, int flags, const struct itimerspec *value,
 		 struct itimerspec *ovalue)
 {
+  /* There's no easy way to implement TFD_TIMER_CANCEL_ON_SET, but
+     we should at least accept the flag. */
   if ((flags & ~(TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET)) != 0)
     {
       set_errno (EINVAL);
diff --git a/winsup/cygwin/timer.h b/winsup/cygwin/timer.h
index 595d9fe..f4c89bc 100644
--- a/winsup/cygwin/timer.h
+++ b/winsup/cygwin/timer.h
@@ -33,8 +33,14 @@ class timer_tracker
   LONG decrement_instances ();
   int clean_and_unhook ();
   LONG64 _disarm_event ();
+  void restart ();
 
  public:
+  void *operator new (size_t size) __attribute__ ((nothrow))
+  { return malloc (size); }
+  void *operator new (size_t, void *p) __attribute__ ((nothrow)) {return p;}
+  void operator delete(void *p) { incygheap (p) ? cfree (p) : free (p); }
+
   timer_tracker (clockid_t, const sigevent *, bool);
   ~timer_tracker ();
   inline bool is_timer_tracker () const { return magic == TT_MAGIC; }



More information about the Cygwin-cvs mailing list