Cygwin: timerfd: rework implementation

timerfd_tracker and timerfd_shared classes:

- Just because handles are shared, we don't have to store them in
  shared memory.  Move share handles into timerfd_tracker class.

- Drop shared instance counter since it's not required anymore.
  timerfd_shared only stores the actual timer data.

- Drop timerfd_shared::create, just set clock id.

- Drop timerfd_shared::dtor, it's not required anymore.

- Drop timerfd_tracker::close, just call dtor where required.

- Rename timerfd_tracker::increment_instances to timerfd_tracker::dup.
  It's the only reason it exists...

- timerfd_tracker::dtor now checks the non-shared pointers for NULL
  before attempting to close them.

- timerfd_tracker::dtor handles decrementing the local instance count
  by itself.

- Add a method timerfd_tracker::init_fixup_after_fork_exec to set
  non-shared pointers to NULL.  Together with the dtor patches it
  fixes a problem with close_on_exec timerfd descriptors.

- Fix a bug in handling the thread synchronization event.  It's
  actually nice to create it before using it...

- Drop using sec_none{_nih} in InitializeObjectAttributes.  It's
  an unnecessary roundabout route just to get a NULL pointer.

- Slightly rework timechange window handling.

- Add more comments to explain what happens.

fhandler_timerfd:

- Drop cnew macro, it just hides what happens.

- fhandler_timerfd::fixup_after_exec now calls
  timerfd_tracker::init_fixup_after_fork_exec first, so a subsequent
  call to timerfd_tracker::dtor only works on valid handles.

- fhandler_timerfd::close directly calls timerfd_tracker::dtor now.

- Drop dtor call in fhandler_timerfd destructor.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

4 files changed, 216 insertions, 240 deletions

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 079385d..0da87e9 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2682,7 +2682,7 @@ class fhandler_timerfd : public fhandler_base
  public:
   fhandler_timerfd ();
   fhandler_timerfd (void *) {}
-  ~fhandler_timerfd ();
+  ~fhandler_timerfd () {}
 
   fhandler_timerfd *is_timerfd () { return this; }
 
diff --git a/winsup/cygwin/fhandler_timerfd.cc b/winsup/cygwin/fhandler_timerfd.cc
index 2a05217..e7e2232 100644
--- a/winsup/cygwin/fhandler_timerfd.cc
+++ b/winsup/cygwin/fhandler_timerfd.cc
@@ -30,21 +30,17 @@ fhandler_timerfd::get_proc_fd_name (char *buf)
 /* 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)
 {
-  timerfd_tracker *tfd = cnew (timerfd_tracker);
+  timerfd_tracker *tfd = (timerfd_tracker *)
+			 ccalloc (HEAP_FHANDLER, 1, sizeof (timerfd_tracker));
   if (!tfd)
     {
       set_errno (ENOMEM);
       return -1;
     }
+  new (tfd) timerfd_tracker ();
   int ret = tfd->create (clock_id);
   if (ret < 0)
     {
@@ -178,7 +174,7 @@ fhandler_timerfd::dup (fhandler_base *child, int flags)
       __try
 	{
 	  timerfd_tracker *tfd = (timerfd_tracker *) fhc->timerid;
-	  tfd->increment_instances ();
+	  tfd->dup ();
 	  ret = 0;
 	}
       __except (EFAULT) {}
@@ -234,8 +230,9 @@ fhandler_timerfd::fixup_after_exec ()
   __try
     {
       timerfd_tracker *tfd = (timerfd_tracker *) timerid;
+      tfd->init_fixup_after_fork_exec ();
       if (close_on_exec ())
-	tfd->decrement_instances ();
+	timerfd_tracker::dtor (tfd);
       else
 	tfd->fixup_after_exec ();
     }
@@ -243,17 +240,6 @@ fhandler_timerfd::fixup_after_exec ()
   __endtry
 }
 
-fhandler_timerfd::~fhandler_timerfd ()
-{
-  __try
-    {
-      timerfd_tracker *tfd = (timerfd_tracker *) timerid;
-      timerfd_tracker::dtor (tfd);
-    }
-  __except (EFAULT) {}
-  __endtry
-}
-
 int
 fhandler_timerfd::close ()
 {
@@ -262,7 +248,7 @@ fhandler_timerfd::close ()
   __try
     {
       timerfd_tracker *tfd = (timerfd_tracker *) timerid;
-      tfd->close ();
+      timerfd_tracker::dtor (tfd);
       ret = 0;
     }
   __except (EFAULT) {}
diff --git a/winsup/cygwin/timerfd.cc b/winsup/cygwin/timerfd.cc
index 2823560..418e71b 100644
--- a/winsup/cygwin/timerfd.cc
+++ b/winsup/cygwin/timerfd.cc
@@ -28,10 +28,9 @@ timerfd_tracker::create_timechange_window ()
   WNDCLASSW wclass = { 0 };
   WCHAR cname[NAME_MAX];
 
-  __small_swprintf (cname, L"Cygwin.timerfd.%u", winpid);
+  __small_swprintf (cname, L"Cygwin.timerfd.%p", this);
   wclass.lpfnWndProc = DefWindowProcW;
-  wclass.hInstance = GetModuleHandle (NULL);
-  wclass.hbrBackground = (HBRUSH) (COLOR_WINDOW + 1);
+  wclass.hInstance = user_data->hmodule;
   wclass.lpszClassName = cname;
   atom = RegisterClassW (&wclass);
   if (!atom)
@@ -39,9 +38,9 @@ timerfd_tracker::create_timechange_window ()
   else
     {
       window = CreateWindowExW (0, cname, cname, WS_POPUP, 0, 0, 0, 0,
-				NULL, NULL, NULL, NULL);
+				NULL, NULL, user_data->hmodule, NULL);
       if (!window)
-	debug_printf ("RegisterClass %E");
+	debug_printf ("CreateWindowEx %E");
     }
 }
 
@@ -51,7 +50,7 @@ timerfd_tracker::delete_timechange_window ()
   if (window)
     DestroyWindow (window);
   if (atom)
-    UnregisterClassW ((LPWSTR) (uintptr_t) atom, GetModuleHandle (NULL));
+    UnregisterClassW ((LPWSTR) (uintptr_t) atom, user_data->hmodule);
 }
 
 void
@@ -59,12 +58,13 @@ timerfd_tracker::handle_timechange_window ()
 {
   MSG msg;
 
-  if (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE) && msg.message != WM_QUIT)
+  while (PeekMessageW (&msg, NULL, 0, 0, PM_REMOVE | PM_QS_POSTMESSAGE)
+	 && msg.message != WM_QUIT)
     {
-      DispatchMessageW(&msg);
+      DispatchMessageW (&msg);
       if (msg.message == WM_TIMECHANGE
 	  && get_clockid () == CLOCK_REALTIME
-	  && (flags () & TFD_CANCEL_FLAGS) == TFD_CANCEL_FLAGS
+	  && (get_flags () & TFD_CANCEL_FLAGS) == TFD_CANCEL_FLAGS
 	  && enter_critical_section ())
 	{
 	  /* make sure to handle each WM_TIMECHANGE only once! */
@@ -84,7 +84,7 @@ DWORD
 timerfd_tracker::thread_func ()
 {
   /* Outer loop: Is the timer armed?  If not, wait for it. */
-  HANDLE armed[2] = { tfd_shared->arm_evt (),
+  HANDLE armed[2] = { arm_evt (),
 		      cancel_evt };
 
   create_timechange_window ();
@@ -105,8 +105,8 @@ timerfd_tracker::thread_func ()
 	}
 
       /* Inner loop: Timer expired?  If not, wait for it. */
-      HANDLE expired[3] = { tfd_shared->timer (),
-			    tfd_shared->disarm_evt (),
+      HANDLE expired[3] = { timer (),
+			    disarm_evt (),
 			    cancel_evt };
 
       while (1)
@@ -133,7 +133,7 @@ timerfd_tracker::thread_func ()
 	  /* Make sure we haven't been abandoned and/or disarmed
 	     in the meantime */
 	  if (expiration_count () == -1LL
-	      || IsEventSignalled (tfd_shared->disarm_evt ()))
+	      || IsEventSignalled (disarm_evt ()))
 	    {
 	      leave_critical_section ();
 	      goto disarmed;
@@ -176,8 +176,7 @@ timerfd_tracker::thread_func ()
 				    || get_clockid () == CLOCK_BOOTTIME_ALARM);
 		  LARGE_INTEGER DueTime = { QuadPart: -get_interval () };
 
-		  NtSetTimer (tfd_shared->timer (), &DueTime, NULL, NULL,
-			      Resume, 0, NULL);
+		  NtSetTimer (timer (), &DueTime, NULL, NULL, Resume, 0, NULL);
 		}
 	    }
 	  /* Arm the expiry object */
@@ -190,7 +189,8 @@ disarmed:
 
 canceled:
   delete_timechange_window ();
-  _my_tls._ctinfo->auto_release ();     /* automatically return the cygthread to the cygthread pool */
+  /* automatically return the cygthread to the cygthread pool */
+  _my_tls._ctinfo->auto_release ();
   return 0;
 }
 
@@ -202,20 +202,40 @@ timerfd_thread (VOID *arg)
 }
 
 int
-timerfd_shared::create (clockid_t clock_id)
+timerfd_tracker::create (clockid_t clock_id)
 {
   int ret;
   NTSTATUS status;
   OBJECT_ATTRIBUTES attr;
 
+  const ACCESS_MASK access = STANDARD_RIGHTS_REQUIRED
+			     | SECTION_MAP_READ | SECTION_MAP_WRITE;
+  SIZE_T vsize = PAGE_SIZE;
+  LARGE_INTEGER sectionsize = { QuadPart: PAGE_SIZE };
+
+  /* Valid clock? */
+  if (!get_clock (clock_id))
+    {
+      ret = -EINVAL;
+      goto err;
+    }
+
+  /* Create shared objects */
+  InitializeObjectAttributes (&attr, NULL, OBJ_INHERIT, NULL, NULL);
+  /* Create shared section */
+  status = NtCreateSection (&tfd_shared_hdl, access, &attr, &sectionsize,
+			    PAGE_READWRITE, SEC_COMMIT, NULL);
+  if (!NT_SUCCESS (status))
+    {
+      ret = -geterrno_from_nt_status (status);
+      goto err;
+    }
   /* Create access mutex */
-  InitializeObjectAttributes (&attr, NULL, OBJ_INHERIT, NULL,
-			      sec_none.lpSecurityDescriptor);
   status = NtCreateMutant (&_access_mtx, MUTEX_ALL_ACCESS, &attr, FALSE);
   if (!NT_SUCCESS (status))
     {
       ret = -geterrno_from_nt_status (status);
-      goto err;
+      goto err_close_tfd_shared_hdl;
     }
   /* Create "timer is armed" event, set to "Unsignaled" at creation time */
   status = NtCreateEvent (&_arm_evt, EVENT_ALL_ACCESS, &attr,
@@ -249,130 +269,93 @@ timerfd_shared::create (clockid_t clock_id)
       ret = -geterrno_from_nt_status (status);
       goto err_close_timer;
     }
-  instance_count = 1;
-  _clockid = clock_id;
-  return 0;
-
-err_close_timer:
-  NtClose (_timer);
-err_close_disarm_evt:
-  NtClose (_disarm_evt);
-err_close_arm_evt:
-  NtClose (_arm_evt);
-err_close_access_mtx:
-  NtClose (_access_mtx);
-err:
-  return ret;
-}
-
-int
-timerfd_tracker::create (clockid_t clock_id)
-{
-  int ret;
-  NTSTATUS status;
-  OBJECT_ATTRIBUTES attr;
-
-  const ACCESS_MASK access = STANDARD_RIGHTS_REQUIRED
-			     | SECTION_MAP_READ | SECTION_MAP_WRITE;
-  SIZE_T vsize = PAGE_SIZE;
-  LARGE_INTEGER sectionsize = { QuadPart: PAGE_SIZE };
-
-  /* Valid clock? */
-  if (!get_clock (clock_id))
-    {
-      ret = -EINVAL;
-      goto err;
-    }
-  /* Create shared section. */
-  InitializeObjectAttributes (&attr, NULL, OBJ_INHERIT, NULL,
-			      sec_none.lpSecurityDescriptor);
-  status = NtCreateSection (&tfd_shared_hdl, access, &attr,
-			    &sectionsize, PAGE_READWRITE,
-			    SEC_COMMIT, NULL);
+  /* Create process-local cancel event for this processes timer thread
+     (has to be recreated after fork/exec)*/
+  InitializeObjectAttributes (&attr, NULL, 0, NULL, NULL);
+  status = NtCreateEvent (&cancel_evt, EVENT_ALL_ACCESS, &attr,
+			  NotificationEvent, FALSE);
   if (!NT_SUCCESS (status))
     {
       ret = -geterrno_from_nt_status (status);
-      goto err;
+      goto err_close_expired_evt;
     }
-  /* Create section mapping (has to be repeated after fork/exec */
-  status = NtMapViewOfSection (tfd_shared_hdl, NtCurrentProcess (),
-			       (void **) &tfd_shared, 0, PAGE_SIZE, NULL,
-			       &vsize, ViewShare, MEM_TOP_DOWN, PAGE_READWRITE);
+  /* Create sync event for this processes timer thread */
+  status = NtCreateEvent (&sync_thr, EVENT_ALL_ACCESS, &attr,
+			  NotificationEvent, FALSE);
   if (!NT_SUCCESS (status))
     {
       ret = -geterrno_from_nt_status (status);
-      goto err_close_tfd_shared_hdl;
+      goto err_close_cancel_evt;
     }
-  /* Create cancel even for this processes timer thread */
-  InitializeObjectAttributes (&attr, NULL, 0, NULL,
-			      sec_none_nih.lpSecurityDescriptor);
-  status = NtCreateEvent (&cancel_evt, EVENT_ALL_ACCESS, &attr,
-			  NotificationEvent, FALSE);
+  /* Create section mapping (has to be recreated after fork/exec) */
+  tfd_shared = NULL;
+  status = NtMapViewOfSection (tfd_shared_hdl, NtCurrentProcess (),
+			       (void **) &tfd_shared, 0, PAGE_SIZE, NULL,
+			       &vsize, ViewShare, 0, PAGE_READWRITE);
   if (!NT_SUCCESS (status))
     {
       ret = -geterrno_from_nt_status (status);
-      goto err_unmap_tfd_shared;
+      goto err_close_sync_thr;
     }
-  ret = tfd_shared->create (clock_id);
-  if (ret < 0)
-    goto err_close_cancel_evt;
+  /* Initialize clock id */
+  set_clockid (clock_id);
+  /* Set our winpid for fixup_after_fork_exec */
   winpid = GetCurrentProcessId ();
+  /* Start timerfd thread */
   new cygthread (timerfd_thread, this, "timerfd", sync_thr);
   return 0;
 
+err_close_sync_thr:
+  NtClose (sync_thr);
 err_close_cancel_evt:
   NtClose (cancel_evt);
-err_unmap_tfd_shared:
-  NtUnmapViewOfSection (NtCurrentProcess (), tfd_shared);
+err_close_expired_evt:
+  NtClose (_expired_evt);
+err_close_timer:
+  NtClose (_timer);
+err_close_disarm_evt:
+  NtClose (_disarm_evt);
+err_close_arm_evt:
+  NtClose (_arm_evt);
+err_close_access_mtx:
+  NtClose (_access_mtx);
 err_close_tfd_shared_hdl:
   NtClose (tfd_shared_hdl);
 err:
   return ret;
 }
 
-/* Return true if this was the last instance of a timerfd, session-wide,
-   false otherwise */
-bool
-timerfd_shared::dtor ()
-{
-  if (instance_count > 0)
-    {
-      return false;
-    }
-  disarm_timer ();
-  NtClose (_timer);
-  NtClose (_arm_evt);
-  NtClose (_disarm_evt);
-  NtClose (_expired_evt);
-  NtClose (_access_mtx);
-  return true;
-}
-
-/* Return true if this was the last instance of a timerfd, session-wide,
+/* Return true if this was the last instance of a timerfd, process-wide,
    false otherwise.  Basically this is a destructor, but one which may
    notify the caller NOT to deleted the object. */
 bool
 timerfd_tracker::dtor ()
 {
-  if (enter_critical_section ())
+  if (!enter_critical_section ())
+    return false;
+  if (decrement_instances () > 0)
+    {
+      leave_critical_section ();
+      return false;
+    }
+  if (cancel_evt)
+    SetEvent (cancel_evt);
+  if (sync_thr)
     {
-      if (local_instance_count > 0)
-	{
-	  leave_critical_section ();
-	  return false;
-	}
-      SetEvent (cancel_evt);
       WaitForSingleObject (sync_thr, INFINITE);
-      if (tfd_shared->dtor ())
-	{
-	  NtUnmapViewOfSection (NtCurrentProcess (), tfd_shared);
-	  NtClose (tfd_shared_hdl);
-	}
-      else
-	leave_critical_section ();
+      NtClose (sync_thr);
     }
-  NtClose (cancel_evt);
-  NtClose (sync_thr);
+  leave_critical_section ();
+  if (tfd_shared)
+    NtUnmapViewOfSection (NtCurrentProcess (), tfd_shared);
+  if (cancel_evt)
+    NtClose (cancel_evt);
+  NtClose (tfd_shared_hdl);
+  NtClose (_expired_evt);
+  NtClose (_timer);
+  NtClose (_disarm_evt);
+  NtClose (_arm_evt);
+  NtClose (_access_mtx);
   return true;
 }
 
@@ -383,13 +366,6 @@ timerfd_tracker::dtor (timerfd_tracker *tfd)
     cfree (tfd);
 }
 
-void
-timerfd_tracker::close ()
-{
-  InterlockedDecrement (&local_instance_count);
-  InterlockedDecrement (&tfd_shared->instance_count);
-}
-
 int
 timerfd_tracker::ioctl_set_ticks (uint64_t new_exp_cnt)
 {
@@ -405,10 +381,21 @@ timerfd_tracker::ioctl_set_ticks (uint64_t new_exp_cnt)
 }
 
 void
+timerfd_tracker::init_fixup_after_fork_exec ()
+{
+  /* Run this only if this is the first call, or all previous calls
+     came from close_on_exec descriptors */
+  if (winpid == GetCurrentProcessId ())
+    return;
+  tfd_shared = NULL;
+  cancel_evt = NULL;
+  sync_thr = NULL;
+}
+
+void
 timerfd_tracker::fixup_after_fork_exec (bool execing)
 {
   NTSTATUS status;
-  PVOID base_address = NULL;
   OBJECT_ATTRIBUTES attr;
   SIZE_T vsize = PAGE_SIZE;
 
@@ -416,24 +403,27 @@ timerfd_tracker::fixup_after_fork_exec (bool execing)
   if (winpid == GetCurrentProcessId ())
     return;
   /* Recreate shared section mapping */
+  tfd_shared = NULL;
   status = NtMapViewOfSection (tfd_shared_hdl, NtCurrentProcess (),
-			       &base_address, 0, PAGE_SIZE, NULL,
+			       (PVOID *) &tfd_shared, 0, PAGE_SIZE, NULL,
 			       &vsize, ViewShare, 0, PAGE_READWRITE);
   if (!NT_SUCCESS (status))
     api_fatal ("Can't recreate shared timerfd section during %s, status %y!",
 	       execing ? "execve" : "fork", status);
-  tfd_shared = (timerfd_shared *) base_address;
-  /* Increment global instance count by the number of instances in this
-     process */
-  InterlockedAdd (&tfd_shared->instance_count, local_instance_count);
-  /* Create cancel even for this processes timer thread */
-  InitializeObjectAttributes (&attr, NULL, 0, NULL,
-			      sec_none_nih.lpSecurityDescriptor);
+  /* Create cancel event for this processes timer thread */
+  InitializeObjectAttributes (&attr, NULL, 0, NULL, NULL);
   status = NtCreateEvent (&cancel_evt, EVENT_ALL_ACCESS, &attr,
 			  NotificationEvent, FALSE);
   if (!NT_SUCCESS (status))
     api_fatal ("Can't recreate timerfd cancel event during %s, status %y!",
 	       execing ? "execve" : "fork", status);
+  /* Create sync event for this processes timer thread */
+  InitializeObjectAttributes (&attr, NULL, 0, NULL, NULL);
+  status = NtCreateEvent (&sync_thr, EVENT_ALL_ACCESS, &attr,
+			  NotificationEvent, FALSE);
+  if (!NT_SUCCESS (status))
+    api_fatal ("Can't recreate timerfd sync event during %s, status %y!",
+	       execing ? "execve" : "fork", status);
   /* Set winpid so we don't run this twice */
   winpid = GetCurrentProcessId ();
   new cygthread (timerfd_thread, this, "timerfd", sync_thr);
@@ -509,7 +499,7 @@ timerfd_tracker::gettime (struct itimerspec *curr_value)
 
   __try
     {
-      if (IsEventSignalled (tfd_shared->disarm_evt ()))
+      if (IsEventSignalled (disarm_evt ()))
 	*curr_value = time_spec ();
       else
 	{
@@ -532,27 +522,28 @@ timerfd_tracker::gettime (struct itimerspec *curr_value)
 }
 
 int
-timerfd_shared::arm_timer (int flags, const struct itimerspec *new_value)
+timerfd_tracker::arm_timer (int flags, const struct itimerspec *new_value)
 {
+  LONG64 interval;
   LONG64 ts;
   NTSTATUS status;
   LARGE_INTEGER DueTime;
   BOOLEAN Resume;
   LONG Period;
 
-  ResetEvent (_disarm_evt);
+  ResetEvent (disarm_evt ());
 
   /* Convert incoming itimerspec into 100ns interval and timestamp */
-  _interval = new_value->it_interval.tv_sec * NS100PERSEC
-	      + (new_value->it_interval.tv_nsec + (NSPERSEC / NS100PERSEC) - 1)
-		/ (NSPERSEC / NS100PERSEC);
+  interval = new_value->it_interval.tv_sec * NS100PERSEC
+	     + (new_value->it_interval.tv_nsec + (NSPERSEC / NS100PERSEC) - 1)
+	       / (NSPERSEC / NS100PERSEC);
   ts = new_value->it_value.tv_sec * NS100PERSEC
 	    + (new_value->it_value.tv_nsec + (NSPERSEC / NS100PERSEC) - 1)
 	      / (NSPERSEC / NS100PERSEC);
-  _flags = flags;
+  set_flags (flags);
   if (flags & TFD_TIMER_ABSTIME)
     {
-      if (_clockid == CLOCK_REALTIME)
+      if (get_clockid () == CLOCK_REALTIME)
 	DueTime.QuadPart = ts + FACTOR;
       else /* non-REALTIME clocks require relative DueTime. */
 	{
@@ -570,17 +561,18 @@ timerfd_shared::arm_timer (int flags, const struct itimerspec *new_value)
       DueTime.QuadPart = -ts;
       ts += get_clock_now ();
     }
-  set_exp_ts (ts);
   time_spec () = *new_value;
+  set_exp_ts (ts);
+  set_interval (interval);
   read_and_reset_expiration_count ();
   /* Note: Advanced Power Settings -> Sleep -> Allow Wake Timers
 	   since W10 1709 */
-  Resume = (_clockid == CLOCK_REALTIME_ALARM
-	    || _clockid == CLOCK_BOOTTIME_ALARM);
-  if (_interval > INT_MAX * (NS100PERSEC / MSPERSEC))
+  Resume = (get_clockid () == CLOCK_REALTIME_ALARM
+	    || get_clockid () == CLOCK_BOOTTIME_ALARM);
+  if (interval > INT_MAX * (NS100PERSEC / MSPERSEC))
     Period = 0;
   else
-    Period = (_interval + (NS100PERSEC / MSPERSEC) - 1)
+    Period = (interval + (NS100PERSEC / MSPERSEC) - 1)
 	     / (NS100PERSEC / MSPERSEC);
   status = NtSetTimer (timer (), &DueTime, NULL, NULL, Resume, Period, NULL);
   if (!NT_SUCCESS (status))
@@ -589,7 +581,7 @@ timerfd_shared::arm_timer (int flags, const struct itimerspec *new_value)
       return -geterrno_from_nt_status (status);
     }
 
-  SetEvent (_arm_evt);
+  SetEvent (arm_evt ());
   return 0;
 }
 
diff --git a/winsup/cygwin/timerfd.h b/winsup/cygwin/timerfd.h
index 66bf784..154be08 100644
--- a/winsup/cygwin/timerfd.h
+++ b/winsup/cygwin/timerfd.h
@@ -14,16 +14,6 @@ details. */
 
 class timerfd_shared
 {
-  HANDLE _access_mtx;		/* controls access to shared data */
-  HANDLE _arm_evt;		/* settimer sets event when timer is armed,
-				   unsets event when timer gets disarmed. */
-  HANDLE _disarm_evt;		/* settimer sets event when timer is armed,
-				   unsets event when timer gets disarmed. */
-  HANDLE _timer;		/* SynchronizationTimer */
-  HANDLE _expired_evt;		/* Signal if timer expired, Unsignal on read. */
-  LONG instance_count;		/* each open fd increments this.
-				   If 0 -> delete timerfd_shared */
-
   clockid_t _clockid;		/* clockid */
   struct itimerspec _time_spec;	/* original incoming itimerspec */
   LONG64 _exp_ts;		/* start timestamp or next expire timestamp
@@ -33,52 +23,29 @@ class timerfd_shared
   int _flags;			/* settime flags */
   DWORD _tc_time;		/* timestamp of the last WM_TIMECHANGE msg */
 
-  int create (clockid_t);
-  bool dtor ();
-
   /* read access methods */
-  HANDLE arm_evt () const { return _arm_evt; }
-  HANDLE disarm_evt () const { return _disarm_evt; }
-  HANDLE timer () const { return _timer; }
-  HANDLE expired_evt () const { return _expired_evt; }
   LONG64 get_clock_now () const { return get_clock (_clockid)->n100secs (); }
   struct itimerspec &time_spec () { return _time_spec; }
-  int flags () const { return _flags; }
+  int get_flags () const { return _flags; }
+  void set_flags (int nflags) { _flags = nflags; }
 
   /* write access methods */
+  void set_clockid (clockid_t clock_id) { _clockid = clock_id; }
   void increment_expiration_count (LONG64 add)
     { InterlockedAdd64 (&_expiration_count, add); }
   void set_expiration_count (LONG64 newval)
     { InterlockedExchange64 (&_expiration_count, newval); }
-  LONG64 read_and_reset_expiration_count ()
-    {
-      LONG64 ret = InterlockedExchange64 (&_expiration_count, 0);
-      if (ret)
-	ResetEvent (_expired_evt);
-      return ret;
-    }
-  bool enter_cs ()
-    {
-      return (WaitForSingleObject (_access_mtx, INFINITE) & ~WAIT_ABANDONED_0)
-	      == WAIT_OBJECT_0;
-    }
-  void leave_cs ()
-    {
-      ReleaseMutex (_access_mtx);
-    }
+  LONG64 reset_expiration_count ()
+    { return InterlockedExchange64 (&_expiration_count, 0); }
   int arm_timer (int, const struct itimerspec *);
   int disarm_timer ()
     {
-      ResetEvent (_arm_evt);
       memset (&_time_spec, 0, sizeof _time_spec);
       _exp_ts = 0;
       _interval = 0;
       /* _flags = 0;  DON'T DO THAT.  Required for TFD_TIMER_CANCEL_ON_SET */
-      NtCancelTimer (timer (), NULL);
-      SetEvent (_disarm_evt);
       return 0;
     }
-  void timer_expired () { SetEvent (_expired_evt); }
   void set_exp_ts (LONG64 ts) { _exp_ts = ts; }
 
   friend class timerfd_tracker;
@@ -86,13 +53,22 @@ class timerfd_shared
 
 class timerfd_tracker		/* cygheap! */
 {
-  HANDLE tfd_shared_hdl;	/* handle auf shared mem */
-  timerfd_shared *tfd_shared;	/* pointer auf shared mem, needs
-				   NtMapViewOfSection in each new process. */
-
+  /* Shared handles */
+  HANDLE tfd_shared_hdl;	/* handle to shared mem */
+  HANDLE _access_mtx;		/* controls access to shared data */
+  HANDLE _arm_evt;		/* settimer sets event when timer is armed,
+				   unsets event when timer gets disarmed. */
+  HANDLE _disarm_evt;		/* settimer sets event when timer is armed,
+				   unsets event when timer gets disarmed. */
+  HANDLE _timer;		/* SynchronizationTimer */
+  HANDLE _expired_evt;		/* Signal if timer expired, Unsignal on read. */
+  /* Process-local handles */
   HANDLE cancel_evt;		/* Signal thread to exit. */
   HANDLE sync_thr;		/* cygthread sync object. */
-  LONG local_instance_count;	/* each open fd increments this.
+  /* pointer to shared timerfd, misc */
+  timerfd_shared *tfd_shared;	/* pointer to shared mem, needs
+				   NtMapViewOfSection in each new process. */
+  LONG instance_count;		/* each open fd increments this.
 				   If 0 -> cancel thread.  */
   DWORD winpid;			/* This is used @ fork/exec time to know if
 				   this tracker already has been fixed up. */
@@ -105,69 +81,91 @@ class timerfd_tracker		/* cygheap! */
 
   bool dtor ();
 
-  bool enter_critical_section () const { return tfd_shared->enter_cs (); }
-  void leave_critical_section () const { tfd_shared->leave_cs (); }
+  bool enter_critical_section ()
+    {
+      return (WaitForSingleObject (_access_mtx, INFINITE) & ~WAIT_ABANDONED_0)
+	      == WAIT_OBJECT_0;
+    }
+  void leave_critical_section ()
+    {
+      ReleaseMutex (_access_mtx);
+    }
 
-  int arm_timer (int flags, const struct itimerspec *new_value) const
-    { return tfd_shared->arm_timer (flags, new_value); }
-  int disarm_timer () const { return tfd_shared->disarm_timer (); }
-  void timer_expired () const { tfd_shared->timer_expired (); }
+  HANDLE arm_evt () const { return _arm_evt; }
+  HANDLE disarm_evt () const { return _disarm_evt; }
+  HANDLE timer () const { return _timer; }
+  HANDLE expired_evt () const { return _expired_evt; }
+  void timer_expired () { SetEvent (_expired_evt); }
+  int arm_timer (int flags, const struct itimerspec *new_value);
+  int disarm_timer ()
+    {
+      ResetEvent (_arm_evt);
+      tfd_shared->disarm_timer ();
+      NtCancelTimer (timer (), NULL);
+      SetEvent (_disarm_evt);
+      return 0;
+    }
+  void timer_expired () const { timer_expired (); }
 
   LONG64 expiration_count () const { return tfd_shared->_expiration_count; }
   void increment_expiration_count (LONG64 add) const
     { tfd_shared->increment_expiration_count (add); }
   void set_expiration_count (LONG64 exp_cnt) const
     { tfd_shared->set_expiration_count ((LONG64) exp_cnt); }
-  LONG64 read_and_reset_expiration_count () const
-    { return tfd_shared->read_and_reset_expiration_count (); }
+  LONG64 read_and_reset_expiration_count ()
+    {
+      LONG64 ret = tfd_shared->reset_expiration_count ();
+      if (ret)
+	ResetEvent (_expired_evt);
+      return ret;
+    }
 
   struct timespec it_value () const
     { return tfd_shared->time_spec ().it_value; }
   struct timespec it_interval () const
     { return tfd_shared->time_spec ().it_interval; }
 
+  void set_clockid (clockid_t clock_id) { tfd_shared->set_clockid (clock_id); }
   clock_t get_clockid () const { return tfd_shared->_clockid; }
   LONG64 get_clock_now () const { return tfd_shared->get_clock_now (); }
   struct itimerspec &time_spec () { return tfd_shared->time_spec (); }
   LONG64 get_exp_ts () const { return tfd_shared->_exp_ts; }
   LONG64 get_interval () const { return tfd_shared->_interval; }
-  int flags () const { return tfd_shared->flags (); }
+  void set_interval (LONG64 intv) { tfd_shared->_interval = intv; }
+  int get_flags () const { return tfd_shared->get_flags (); }
+  void set_flags (int nflags) { tfd_shared->set_flags (nflags); }
   DWORD tc_time () const { return tfd_shared->_tc_time; }
   void set_tc_time (DWORD new_time) { tfd_shared->_tc_time = new_time; }
 
   void set_exp_ts (LONG64 ts) const { tfd_shared->set_exp_ts (ts); }
+  LONG decrement_instances () { return InterlockedDecrement (&instance_count); }
 
  public:
   void *operator new (size_t, void *p) __attribute__ ((nothrow)) {return p;}
   timerfd_tracker ()
-  : tfd_shared_hdl (NULL), tfd_shared (NULL), cancel_evt (NULL),
-    sync_thr (NULL), local_instance_count (1), winpid (0), window (NULL),
-    atom (0) {}
-  int create (clockid_t);
-  int gettime (struct itimerspec *);
-  int settime (int, const struct itimerspec *, struct itimerspec *);
-  static void dtor (timerfd_tracker *);
-  void close ();
-  int ioctl_set_ticks (uint64_t);
+  : tfd_shared_hdl (NULL), _access_mtx (NULL), _arm_evt (NULL),
+    _disarm_evt (NULL), cancel_evt (NULL), sync_thr (NULL), tfd_shared (NULL),
+    instance_count (1), winpid (0), window (NULL), atom (0) {}
+
+  void init_fixup_after_fork_exec ();
   void fixup_after_fork_exec (bool);
-  void fixup_after_fork () { fixup_after_fork_exec (false); }
-  void fixup_after_exec () { fixup_after_fork_exec (true); }
-  HANDLE get_timerfd_handle () const { return tfd_shared->expired_evt (); }
-  HANDLE get_disarm_evt () const { return tfd_shared->disarm_evt (); }
-  LONG64 wait (bool);
-  void increment_global_instances ()
-    { InterlockedIncrement (&tfd_shared->instance_count); }
-  void increment_instances ()
+  void fixup_after_fork ()
     {
-      InterlockedIncrement (&tfd_shared->instance_count);
-      InterlockedIncrement (&local_instance_count);
-    }
-  void decrement_instances ()
-    {
-      InterlockedDecrement (&tfd_shared->instance_count);
-      InterlockedDecrement (&local_instance_count);
+      init_fixup_after_fork_exec ();
+      fixup_after_fork_exec (false);
     }
+  void fixup_after_exec () { fixup_after_fork_exec (true); }
+
+  void dup () { InterlockedIncrement (&instance_count); }
+  HANDLE get_timerfd_handle () const { return expired_evt (); }
+  LONG64 wait (bool);
+  int ioctl_set_ticks (uint64_t);
 
+  int create (clockid_t);
+  int gettime (struct itimerspec *);
+  int settime (int, const struct itimerspec *, struct itimerspec *);
+
+  static void dtor (timerfd_tracker *);
   DWORD thread_func ();
 };