Improve w32notify notifications

* src/w32notify.c (DIRWATCH_BUFFER_SIZE): New macro.
(struct notification): 'terminate' is now a HANDLE.
(send_notifications): Argument is now a pointer to a
notification.  Don't loop waiting for the notification to be
acknowledged by the main thread; instead, just add the
notification to the linked list of notifications waiting to be
acknowledged.
(watch_end): Don't close the directory handle.
(watch_completion): Allocate a new notification structure to be
added to the notifications set.  Call ReadDirectoryChangesW
immediately after adding the new notification, and before sending
a message to the main thread about them.
(watch_worker): Don't loop calling ReadDirectoryChangesW; instead,
call it just once -- it will be called again in watch_completion.
Loop waiting for the main thread's indication to terminate.
(start_watching): Create the event to be used to indicate to the
worker thread that its should terminate.
(remove_watch): Indicate to the worker thread that it should
terminate.
* src/w32term.c (queue_notifications): Loop over all the
notifications in the linked list, processing all of them in one
go.
* src/w32inevt.c (handle_file_notifications): Loop over all the
notifications in the linked list.
* src/w32xfns.c (init_crit): Initialize the linked list of file
notifications.
(delete_crit): Free the linked list of file notifications,
including any unprocessed notifications left in it.
* src/w32term.h (struct notifications_se): New struct.

* test/lisp/filenotify-tests.el (file-notify-test02-events)
(file-notify-test05-dir-validity): Add read-event calls to
facilitate event recognition by the main thread in batch mode.

6 files changed, 360 insertions, 250 deletions

diff --git a/src/w32inevt.c b/src/w32inevt.c
index a33f82dc7d..2269d31805 100644
--- a/src/w32inevt.c
+++ b/src/w32inevt.c
@@ -620,70 +620,89 @@ maybe_generate_resize_event (void)
 int
 handle_file_notifications (struct input_event *hold_quit)
 {
-  BYTE *p = file_notifications;
-  FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p;
-  const DWORD min_size
-    = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t);
-  struct input_event inev;
+  struct notifications_set *ns = NULL;
   int nevents = 0;
+  int done = 0;
 
   /* We cannot process notification before Emacs is fully initialized,
      since we need the UTF-16LE coding-system to be set up.  */
   if (!initialized)
     {
-      notification_buffer_in_use = 0;
       return nevents;
     }
 
-  enter_crit ();
-  if (notification_buffer_in_use)
+  while (!done)
     {
-      DWORD info_size = notifications_size;
-      Lisp_Object cs = Qutf_16le;
-      Lisp_Object obj = w32_get_watch_object (notifications_desc);
-
-      /* notifications_size could be zero when the buffer of
-	 notifications overflowed on the OS level, or when the
-	 directory being watched was itself deleted.  Do nothing in
-	 that case.  */
-      if (info_size
-	  && !NILP (obj) && CONSP (obj))
-	{
-	  Lisp_Object callback = XCDR (obj);
+      ns = NULL;
 
-	  EVENT_INIT (inev);
+      /* Find out if there is a record available in the linked list of
+	 notifications sets.  If so, unlink te set from the linked list.
+	 Use the critical section.  */
+      enter_crit ();
+      if (notifications_set_head->next != notifications_set_head)
+	{
+	  ns = notifications_set_head->next;
+	  ns->prev->next = ns->next;
+	  ns->next->prev = ns->prev;
+	}
+      else
+	done = 1;
+      leave_crit();
 
-	  while (info_size >= min_size)
+      if (ns)
+	{
+	  BYTE *p = ns->notifications;
+	  FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p;
+	  const DWORD min_size
+	    = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t);
+	  struct input_event inev;
+	  DWORD info_size = ns->size;
+	  Lisp_Object cs = Qutf_16le;
+	  Lisp_Object obj = w32_get_watch_object (ns->desc);
+
+	  /* notifications size could be zero when the buffer of
+	     notifications overflowed on the OS level, or when the
+	     directory being watched was itself deleted.  Do nothing in
+	     that case.  */
+	  if (info_size
+	      && !NILP (obj) && CONSP (obj))
 	    {
-	      Lisp_Object utf_16_fn
-		= make_unibyte_string ((char *)fni->FileName,
-				       fni->FileNameLength);
-	      /* Note: mule-conf is preloaded, so utf-16le must
-		 already be defined at this point.  */
-	      Lisp_Object fname
-		= code_convert_string_norecord (utf_16_fn, cs, 0);
-	      Lisp_Object action = lispy_file_action (fni->Action);
-
-	      inev.kind = FILE_NOTIFY_EVENT;
-	      inev.timestamp = GetTickCount ();
-	      inev.modifiers = 0;
-	      inev.frame_or_window = callback;
-	      inev.arg = Fcons (action, fname);
-	      inev.arg = list3 (make_pointer_integer (notifications_desc),
-				action, fname);
-	      kbd_buffer_store_event_hold (&inev, hold_quit);
-	      nevents++;
-
-	      if (!fni->NextEntryOffset)
-		break;
-	      p += fni->NextEntryOffset;
-	      fni = (PFILE_NOTIFY_INFORMATION)p;
-	      info_size -= fni->NextEntryOffset;
+	      Lisp_Object callback = XCDR (obj);
+
+	      EVENT_INIT (inev);
+
+	      while (info_size >= min_size)
+		{
+		  Lisp_Object utf_16_fn
+		    = make_unibyte_string ((char *)fni->FileName,
+					   fni->FileNameLength);
+		  /* Note: mule-conf is preloaded, so utf-16le must
+		     already be defined at this point.  */
+		  Lisp_Object fname
+		    = code_convert_string_norecord (utf_16_fn, cs, 0);
+		  Lisp_Object action = lispy_file_action (fni->Action);
+
+		  inev.kind = FILE_NOTIFY_EVENT;
+		  inev.timestamp = GetTickCount ();
+		  inev.modifiers = 0;
+		  inev.frame_or_window = callback;
+		  inev.arg = Fcons (action, fname);
+		  inev.arg = list3 (make_pointer_integer (ns->desc),
+				    action, fname);
+		  kbd_buffer_store_event_hold (&inev, hold_quit);
+		  nevents++;
+		  if (!fni->NextEntryOffset)
+		    break;
+		  p += fni->NextEntryOffset;
+		  fni = (PFILE_NOTIFY_INFORMATION)p;
+		  info_size -= fni->NextEntryOffset;
+		}
 	    }
+	  /* Free this notification set.  */
+	  free (ns->notifications);
+	  free (ns);
 	}
-      notification_buffer_in_use = 0;
     }
-  leave_crit ();
   return nevents;
 }
 #else  /* !HAVE_W32NOTIFY */
diff --git a/src/w32notify.c b/src/w32notify.c
index 586c2062f6..54d9bcc189 100644
--- a/src/w32notify.c
+++ b/src/w32notify.c
@@ -22,27 +22,30 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
    For each watch request, we launch a separate worker thread.  The
    worker thread runs the watch_worker function, which issues an
-   asynchronous call to ReadDirectoryChangesW, and then waits in
-   SleepEx for that call to complete.  Waiting in SleepEx puts the
-   thread in an "alertable" state, so it wakes up when either (a) the
-   call to ReadDirectoryChangesW completes, or (b) the main thread
-   instructs the worker thread to terminate by sending it an APC, see
-   below.
+   asynchronous call to ReadDirectoryChangesW, and then calls
+   WaitForSingleObjectEx to wait that an event be signaled
+   to terminate the thread.
+   Waiting with WaitForSingleObjectEx puts the thread in an
+   "alertable" state, so it wakes up when either (a) the call to
+   ReadDirectoryChangesW completes, or (b) the main thread instructs
+   the worker thread to terminate by signaling an event, see below.
 
    When the ReadDirectoryChangesW call completes, its completion
    routine watch_completion is automatically called.  watch_completion
-   stashes the received file events in a buffer used to communicate
-   them to the main thread (using a critical section, so that several
-   threads could use the same buffer), posts a special message,
-   WM_EMACS_FILENOTIFY, to the Emacs's message queue, and returns.
-   That causes the SleepEx function call inside watch_worker to
-   return, and watch_worker then issues another call to
-   ReadDirectoryChangesW.  (Except when it does not, see below.)
+   stashes the received file events in a linked list used to
+   communicate them to the main thread (using a critical section, so
+   that several threads could alter the same linked list), posts a
+   special message, WM_EMACS_FILENOTIFY, to the Emacs's message queue,
+   and returns.  That causes the WaitForSingleObjectEx function call
+   inside watch_worker to return, but the thread won't terminate until
+   the event telling to do so will be signaled.  The completion
+   routine issued another call to ReadDirectoryChangesW as quickly as
+   possible.  (Except when it does not, see below.)
 
    In a GUI session, the WM_EMACS_FILENOTIFY message posted to the
    message queue gets dispatched to the main Emacs window procedure,
    which queues it for processing by w32_read_socket.  When
-   w32_read_socket sees this message, it accesses the buffer with file
+   w32_read_socket sees this message, it accesses the linked list with file
    notifications (using a critical section), extracts the information,
    converts it to a series of FILE_NOTIFY_EVENT events, and stuffs
    them into the input event queue to be processed by keyboard.c input
@@ -53,7 +56,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    procedures in console programs.  That message wakes up
    MsgWaitForMultipleObjects inside sys_select, which then signals to
    its caller that some keyboard input is available.  This causes
-   w32_console_read_socket to be called, which accesses the buffer
+   w32_console_read_socket to be called, which accesses the linked list
    with file notifications and stuffs them into the input event queue
    for keyboard.c to process.
 
@@ -62,24 +65,21 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    bound to a command.  The default binding is file-notify-handle-event,
    defined on subr.el.
 
-   After w32_read_socket or w32_console_read_socket are done
-   processing the notifications, they reset a flag signaling to all
-   watch worker threads that the notifications buffer is available for
-   more input.
+   Routines w32_read_socket or w32_console_read_socket process notifications
+   sets as long as some are available.
 
    When the watch is removed by a call to w32notify-rm-watch, the main
-   thread requests that the worker thread terminates by queuing an APC
-   for the worker thread.  The APC specifies the watch_end function to
-   be called.  watch_end calls CancelIo on the outstanding
-   ReadDirectoryChangesW call and closes the handle on which the
-   watched directory was open.  When watch_end returns, the
-   watch_completion function is called one last time with the
-   ERROR_OPERATION_ABORTED status, which causes it to clean up and set
-   a flag telling watch_worker to exit without issuing another
-   ReadDirectoryChangesW call.  Since watch_worker is the thread
-   procedure of the worker thread, exiting it causes the thread to
-   exit.  The main thread waits for some time for the worker thread to
-   exit, and if it doesn't, terminates it forcibly.  */
+   thread requests that the worker thread terminates by signaling the
+   appropriate event and queuing an APC for the worker thread.  The
+   APC specifies the watch_end function to be called.  watch_end calls
+   CancelIo on the outstanding ReadDirectoryChangesW call.  When
+   watch_end returns, the watch_completion function is called one last
+   time with the ERROR_OPERATION_ABORTED status, which causes it to
+   clean up and set a flag telling watch_worker to exit without
+   issuing another ReadDirectoryChangesW call.  Since watch_worker is
+   the thread procedure of the worker thread, exiting it causes the
+   thread to exit.  The main thread waits for some time for the worker
+   thread to exit, and if it doesn't, terminates it forcibly.  */
 
 #include <stddef.h>
 #include <errno.h>
@@ -98,6 +98,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "frame.h"	/* needed by termhooks.h */
 #include "termhooks.h"	/* for FILE_NOTIFY_EVENT */
 
+#define DIRWATCH_BUFFER_SIZE 16384
 #define DIRWATCH_SIGNATURE 0x01233210
 
 struct notification {
@@ -108,73 +109,52 @@ struct notification {
   char *watchee;	/* the file we are interested in, UTF-8 encoded */
   HANDLE dir;		/* handle to the watched directory */
   HANDLE thr;		/* handle to the thread that watches */
-  volatile int terminate; /* if non-zero, request for the thread to terminate */
+  HANDLE terminate;     /* event signaling the thread to terminate */
   unsigned signature;
 };
 
 /* Used for communicating notifications to the main thread.  */
-volatile int notification_buffer_in_use;
-BYTE file_notifications[16384];
-DWORD notifications_size;
-void *notifications_desc;
+struct notifications_set *notifications_set_head;
 
 static Lisp_Object watch_list;
 
 /* Signal to the main thread that we have file notifications for it to
    process.  */
 static void
-send_notifications (BYTE *info, DWORD info_size, void *desc,
-		    volatile int *terminate)
+send_notifications (struct notifications_set *ns)
 {
   int done = 0;
   struct frame *f = SELECTED_FRAME ();
 
-  /* A single buffer is used to communicate all notifications to the
-     main thread.  Since both the main thread and several watcher
-     threads could be active at the same time, we use a critical area
-     and an "in-use" flag to synchronize them.  A watcher thread can
-     only put its notifications in the buffer if it acquires the
-     critical area and finds the "in-use" flag reset.  The main thread
-     resets the flag after it is done processing notifications.
-
-     FIXME: is there a better way of dealing with this?  */
-  while (!done && !*terminate)
-    {
+  /* We add the current notification set to the linked list.  Use the
+     critical section to make sure only one thread will access the
+     linked list. */
       enter_crit ();
-      if (!notification_buffer_in_use)
-	{
-	  if (info_size)
-	    memcpy (file_notifications, info,
-		    min (info_size, sizeof (file_notifications)));
-	  notifications_size = min (info_size, sizeof (file_notifications));
-	  notifications_desc = desc;
-	  /* If PostMessage fails, the message queue is full.  If that
-	     happens, the last thing they will worry about is file
-	     notifications.  So we effectively discard the
-	     notification in that case.  */
-	  if ((FRAME_TERMCAP_P (f)
-	       /* We send the message to the main (a.k.a. "Lisp")
-		  thread, where it will wake up MsgWaitForMultipleObjects
-		  inside sys_select, causing it to report that there's
-		  some keyboard input available.  This will in turn cause
-		  w32_console_read_socket to be called, which will pick
-		  up the file notifications.  */
-	       && PostThreadMessage (dwMainThreadId, WM_EMACS_FILENOTIFY, 0, 0))
-	      || (FRAME_W32_P (f)
-		  && PostMessage (FRAME_W32_WINDOW (f),
-				  WM_EMACS_FILENOTIFY, 0, 0))
-	      /* When we are running in batch mode, there's no one to
-		 send a message, so we just signal the data is
-		 available and hope sys_select will be called soon and
-		 will read the data.  */
-	      || (FRAME_INITIAL_P (f) && noninteractive))
-	    notification_buffer_in_use = 1;
-	  done = 1;
-	}
-      leave_crit ();
-      if (!done)
-	Sleep (5);
-    }
+  ns->next = notifications_set_head;
+  ns->prev = notifications_set_head->prev;
+  ns->prev->next = ns;
+  notifications_set_head->prev = ns;
+  leave_crit();
+
+  /* If PostMessage fails, the message queue is full.  If that
+     happens, the last thing they will worry about is file
+     notifications.  So we effectively discard the notification in
+     that case.  */
+  if (FRAME_TERMCAP_P (f))
+    /* We send the message to the main (a.k.a. "Lisp") thread, where
+       it will wake up MsgWaitForMultipleObjects inside sys_select,
+       causing it to report that there's some keyboard input
+       available.  This will in turn cause w32_console_read_socket to
+       be called, which will pick up the file notifications.  */
+    PostThreadMessage (dwMainThreadId, WM_EMACS_FILENOTIFY, 0, 0);
+  else if (FRAME_W32_P (f))
+    PostMessage (FRAME_W32_WINDOW (f),
+                 WM_EMACS_FILENOTIFY, 0, 0);
+  /* When we are running in batch mode, there's no one to send a
+     message, so we just signal the data is available and hope
+     sys_select will be called soon and will read the data.  */
+  else if (FRAME_INITIAL_P (f) && noninteractive)
+    ;
 }
 
 /* An APC routine to cancel outstanding directory watch.  Invoked by
@@ -188,10 +168,7 @@ watch_end (ULONG_PTR arg)
   HANDLE hdir = (HANDLE)arg;
 
   if (hdir && hdir != INVALID_HANDLE_VALUE)
-    {
-      CancelIo (hdir);
-      CloseHandle (hdir);
-    }
+    CancelIo (hdir);
 }
 
 /* A completion routine (a.k.a. "APC function") for handling events
@@ -202,13 +179,19 @@ VOID CALLBACK
 watch_completion (DWORD status, DWORD bytes_ret, OVERLAPPED *io_info)
 {
   struct notification *dirwatch;
+  DWORD _bytes;
+  struct notifications_set *ns = NULL;
+  BOOL terminate = FALSE;
 
   /* Who knows what happened?  Perhaps the OVERLAPPED structure was
      freed by someone already?  In any case, we cannot do anything
      with this request, so just punt and skip it.  FIXME: should we
      raise the 'terminate' flag in this case?  */
   if (!io_info)
-    return;
+    {
+      DebPrint(("watch_completion: io_info is null.\n"));
+      return;
+    }
 
   /* We have a pointer to our dirwatch structure conveniently stashed
      away in the hEvent member of the OVERLAPPED struct.  According to
@@ -216,26 +199,69 @@ watch_completion (DWORD status, DWORD bytes_ret, OVERLAPPED *io_info)
      of the OVERLAPPED structure is not used by the system, so you can
      use it yourself."  */
   dirwatch = (struct notification *)io_info->hEvent;
+
   if (status == ERROR_OPERATION_ABORTED)
     {
       /* We've been called because the main thread told us to issue
 	 CancelIo on the directory we watch, and watch_end did so.
-	 The directory handle is already closed.  We should clean up
-	 and exit, signaling to the thread worker routine not to
-	 issue another call to ReadDirectoryChangesW.  Note that we
-	 don't free the dirwatch object itself nor the memory consumed
-	 by its buffers; this is done by the main thread in
-	 remove_watch.  Calling malloc/free from a thread other than
-	 the main thread is a no-no.  */
-      dirwatch->dir = NULL;
-      dirwatch->terminate = 1;
+         We must exit, without issuing another call to
+         ReadDirectoryChangesW. */
+      return;
     }
-  else
+
+  /* We allocate a new set of notifications to be linked to the linked
+     list of notifications set.  This will be processed by Emacs event
+     loop in the main thread.  We need to duplicate the notifications
+     buffer, but not the dirwatch structure.  */
+
+  /* Implementation note: In general, allocating memory in non-main
+     threads is a no-no in Emacs.  We certainly cannot call xmalloc
+     and friends, because it can longjmp when allocation fails, which
+     will crash Emacs because the jmp_buf is set up to a location on
+     the main thread's stack.  However, we can call 'malloc' directly,
+     since that is redirected to HeapAlloc that uses our private heap,
+     see w32heap.c, and that is thread-safe.  */
+  ns = malloc (sizeof(struct notifications_set));
+  if (ns)
+    {
+      memset (ns, 0, sizeof(struct notifications_set));
+      ns->notifications = malloc (bytes_ret);
+      if (ns->notifications)
+	{
+	  memcpy (ns->notifications, dirwatch->buf, bytes_ret);
+	  ns->size = bytes_ret;
+	  ns->desc = dirwatch;
+	}
+      else
+	{
+	  free (ns);
+	  ns = NULL;
+	}
+    }
+  if (ns == NULL)
+    DebPrint(("Out of memory.  Notifications lost."));
+
+  /* Calling ReadDirectoryChangesW quickly to watch again for new
+     notifications.  */
+  if (!ReadDirectoryChangesW (dirwatch->dir, dirwatch->buf,
+			      DIRWATCH_BUFFER_SIZE, dirwatch->subtree,
+			      dirwatch->filter, &_bytes, dirwatch->io_info,
+			      watch_completion))
     {
-      /* Tell the main thread we have notifications for it.  */
-      send_notifications (dirwatch->buf, bytes_ret, dirwatch,
-			  &dirwatch->terminate);
+      DebPrint (("ReadDirectoryChangesW error: %lu\n", GetLastError ()));
+      /* If this call fails, it means that the directory is not
+         watchable any more.  We need to terminate the worker thread.
+         Still, we will wait until the current notifications have been
+         sent to the main thread.  */
+      terminate = TRUE;
     }
+
+  if (ns)
+    send_notifications(ns);
+
+  /* If we were asked to terminate the thread, then fire the event. */
+  if (terminate)
+    SetEvent(dirwatch->terminate);
 }
 
 /* Worker routine for the watch thread.  */
@@ -243,42 +269,43 @@ static DWORD WINAPI
 watch_worker (LPVOID arg)
 {
   struct notification *dirwatch = (struct notification *)arg;
+  BOOL bErr;
+  DWORD _bytes = 0;
+  DWORD status;
+
+  if (dirwatch->dir)
+    {
+      bErr = ReadDirectoryChangesW (dirwatch->dir, dirwatch->buf,
+				    DIRWATCH_BUFFER_SIZE, dirwatch->subtree,
+				    dirwatch->filter, &_bytes,
+				    dirwatch->io_info, watch_completion);
+      if (!bErr)
+	{
+	  DebPrint (("ReadDirectoryChangesW: %lu\n", GetLastError ()));
+	  /* We cannot remove the dirwatch object from watch_list,
+	     because we are in a separate thread.  For the same
+	     reason, we also cannot free memory consumed by the
+	     buffers allocated for the dirwatch object.  So we close
+	     the directory handle, but do not free the object itself
+	     or its buffers.  We also don't touch the signature.  This
+	     way, remove_watch can still identify the object, remove
+	     it, and free its memory.  */
+	  CloseHandle (dirwatch->dir);
+	  dirwatch->dir = NULL;
+	  return 1;
+	}
+    }
 
   do {
-    BOOL status;
-    DWORD bytes_ret = 0;
-
-    if (dirwatch->dir)
-      {
-	status = ReadDirectoryChangesW (dirwatch->dir, dirwatch->buf, 16384,
-					dirwatch->subtree, dirwatch->filter,
-					&bytes_ret,
-					dirwatch->io_info, watch_completion);
-	if (!status)
-	  {
-	    DebPrint (("watch_worker, abnormal exit: %lu\n", GetLastError ()));
-	    /* We cannot remove the dirwatch object from watch_list,
-	       because we are in a separate thread.  For the same
-	       reason, we also cannot free memory consumed by the
-	       buffers allocated for the dirwatch object.  So we close
-	       the directory handle, but do not free the object itself
-	       or its buffers.  We also don't touch the signature.
-	       This way, remove_watch can still identify the object,
-	       remove it, and free its memory.  */
-	    CloseHandle (dirwatch->dir);
-	    dirwatch->dir = NULL;
-	    return 1;
-	  }
-      }
-    /* Sleep indefinitely until awoken by the I/O completion, which
-       could be either a change notification or a cancellation of the
-       watch.  */
-    SleepEx (INFINITE, TRUE);
-  } while (!dirwatch->terminate);
+    status = WaitForSingleObjectEx(dirwatch->terminate, INFINITE, TRUE);
+  } while (status == WAIT_IO_COMPLETION);
+
+  /* The thread is about to terminate, so we clean up the dir handle.  */
+  CloseHandle (dirwatch->dir);
+  dirwatch->dir = NULL;
 
   return 0;
 }
-
 /* Launch a thread to watch changes to FILE in a directory open on
    handle HDIR.  */
 static struct notification *
@@ -287,7 +314,7 @@ start_watching (const char *file, HANDLE hdir, BOOL subdirs, DWORD flags)
   struct notification *dirwatch = xzalloc (sizeof (struct notification));
 
   dirwatch->signature = DIRWATCH_SIGNATURE;
-  dirwatch->buf = xmalloc (16384);
+  dirwatch->buf = xmalloc (DIRWATCH_BUFFER_SIZE);
   dirwatch->io_info = xzalloc (sizeof(OVERLAPPED));
   /* Stash a pointer to dirwatch structure for use by the completion
      routine.  According to MSDN documentation of ReadDirectoryChangesW:
@@ -297,7 +324,9 @@ start_watching (const char *file, HANDLE hdir, BOOL subdirs, DWORD flags)
   dirwatch->subtree = subdirs;
   dirwatch->filter = flags;
   dirwatch->watchee = xstrdup (file);
-  dirwatch->terminate = 0;
+
+  dirwatch->terminate = CreateEvent(NULL, FALSE, FALSE, NULL);
+
   dirwatch->dir = hdir;
 
   /* See w32proc.c where it calls CreateThread for the story behind
@@ -307,11 +336,11 @@ start_watching (const char *file, HANDLE hdir, BOOL subdirs, DWORD flags)
 
   if (!dirwatch->thr)
     {
+      CloseHandle(dirwatch->terminate);
       xfree (dirwatch->buf);
       xfree (dirwatch->io_info);
       xfree (dirwatch->watchee);
       xfree (dirwatch);
-      dirwatch = NULL;
     }
   return dirwatch;
 }
@@ -370,7 +399,10 @@ add_watch (const char *parent_dir, const char *file, BOOL subdirs, DWORD flags)
     return NULL;
 
   if ((dirwatch = start_watching (file, hdir, subdirs, flags)) == NULL)
-    CloseHandle (hdir);
+    {
+      CloseHandle (hdir);
+      dirwatch->dir = NULL;
+    }
 
   return dirwatch;
 }
@@ -383,7 +415,7 @@ remove_watch (struct notification *dirwatch)
     {
       int i;
       BOOL status;
-      DWORD exit_code, err;
+      DWORD exit_code = 0, err;
 
       /* Only the thread that issued the outstanding I/O call can call
 	 CancelIo on it.  (CancelIoEx is available only since Vista.)
@@ -391,12 +423,10 @@ remove_watch (struct notification *dirwatch)
 	 to terminate.  */
       if (!QueueUserAPC (watch_end, dirwatch->thr, (ULONG_PTR)dirwatch->dir))
 	DebPrint (("QueueUserAPC failed (%lu)!\n", GetLastError ()));
-      /* We also set the terminate flag, for when the thread is
-	 waiting on the critical section that never gets acquired.
-	 FIXME: is there a cleaner method?  Using SleepEx there is a
-	 no-no, as that will lead to recursive APC invocations and
-	 stack overflow.  */
-      dirwatch->terminate = 1;
+
+      /* We also signal the thread that it can terminate.  */
+      SetEvent(dirwatch->terminate);
+
       /* Wait for the thread to exit.  FIXME: is there a better method
 	 that is not overly complex?  */
       for (i = 0; i < 50; i++)
@@ -406,11 +436,13 @@ remove_watch (struct notification *dirwatch)
 	    break;
 	  Sleep (10);
 	}
+
       if ((status == FALSE && (err = GetLastError ()) == ERROR_INVALID_HANDLE)
 	  || exit_code == STILL_ACTIVE)
 	{
 	  if (!(status == FALSE && err == ERROR_INVALID_HANDLE))
 	    {
+              DebPrint(("Forcing thread termination.\n"));
 	      TerminateThread (dirwatch->thr, 0);
 	      if (dirwatch->dir)
 		CloseHandle (dirwatch->dir);
@@ -423,11 +455,11 @@ remove_watch (struct notification *dirwatch)
 	  CloseHandle (dirwatch->thr);
 	  dirwatch->thr = NULL;
 	}
+      CloseHandle(dirwatch->terminate);
       xfree (dirwatch->buf);
       xfree (dirwatch->io_info);
       xfree (dirwatch->watchee);
       xfree (dirwatch);
-
       return 0;
     }
   else
diff --git a/src/w32term.c b/src/w32term.c
index 62ad4eb086..8955ce26b4 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -3211,71 +3211,85 @@ static void
 queue_notifications (struct input_event *event, W32Msg *msg, struct frame *f,
 		     int *evcount)
 {
-  BYTE *p = file_notifications;
-  FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p;
-  const DWORD min_size
-    = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t);
+  struct notifications_set *ns = NULL;
   Lisp_Object frame;
+  int done = 0;
 
   /* We cannot process notification before Emacs is fully initialized,
      since we need the UTF-16LE coding-system to be set up.  */
   if (!initialized)
-    {
-      notification_buffer_in_use = 0;
-      return;
-    }
+    return;
 
   XSETFRAME (frame, f);
 
-  enter_crit ();
-  if (notification_buffer_in_use)
+  while (!done)
     {
-      DWORD info_size = notifications_size;
-      Lisp_Object cs = Qutf_16le;
-      Lisp_Object obj = w32_get_watch_object (notifications_desc);
-
-      /* notifications_size could be zero when the buffer of
-	 notifications overflowed on the OS level, or when the
-	 directory being watched was itself deleted.  Do nothing in
-	 that case.  */
-      if (info_size
-	  && !NILP (obj) && CONSP (obj))
+      ns = NULL;
+
+      /* Find out if there is a record available in the linked list of
+	 notifications sets.  If so, unlink the set from the linked
+	 list.  Use critical section.  */
+      enter_crit ();
+      if (notifications_set_head->next != notifications_set_head)
 	{
-	  Lisp_Object callback = XCDR (obj);
+	  ns = notifications_set_head->next;
+	  ns->prev->next = ns->next;
+	  ns->next->prev = ns->prev;
+	}
+      else
+	done = 1;
+      leave_crit();
 
-	  while (info_size >= min_size)
+      if (ns)
+	{
+	  BYTE *p = ns->notifications;
+	  FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p;
+	  const DWORD min_size
+	    = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t);
+	  DWORD info_size = ns->size;
+	  Lisp_Object cs = Qutf_16le;
+	  Lisp_Object obj = w32_get_watch_object (ns->desc);
+
+	  /* notifications size could be zero when the buffer of
+	     notifications overflowed on the OS level, or when the
+	     directory being watched was itself deleted.  Do nothing in
+	     that case.  */
+	  if (info_size
+	      && !NILP (obj) && CONSP (obj))
 	    {
-	      Lisp_Object utf_16_fn
-		= make_unibyte_string ((char *)fni->FileName,
-				       fni->FileNameLength);
-	      /* Note: mule-conf is preloaded, so utf-16le must
-		 already be defined at this point.  */
-	      Lisp_Object fname
-		= code_convert_string_norecord (utf_16_fn, cs, 0);
-	      Lisp_Object action = lispy_file_action (fni->Action);
-
-	      event->kind = FILE_NOTIFY_EVENT;
-	      event->timestamp = msg->msg.time;
-	      event->modifiers = 0;
-	      event->frame_or_window = callback;
-	      event->arg = list3 (make_pointer_integer (notifications_desc),
-				  action, fname);
-	      kbd_buffer_store_event (event);
-	      (*evcount)++;
-
-	      if (!fni->NextEntryOffset)
-		break;
-	      p += fni->NextEntryOffset;
-	      fni = (PFILE_NOTIFY_INFORMATION)p;
-	      info_size -= fni->NextEntryOffset;
+	      Lisp_Object callback = XCDR (obj);
+
+	      while (info_size >= min_size)
+		{
+		  Lisp_Object utf_16_fn
+		    = make_unibyte_string ((char *)fni->FileName,
+					   fni->FileNameLength);
+		  /* Note: mule-conf is preloaded, so utf-16le must
+		     already be defined at this point.  */
+		  Lisp_Object fname
+		    = code_convert_string_norecord (utf_16_fn, cs, 0);
+		  Lisp_Object action = lispy_file_action (fni->Action);
+
+		  event->kind = FILE_NOTIFY_EVENT;
+		  event->timestamp = msg->msg.time;
+		  event->modifiers = 0;
+		  event->frame_or_window = callback;
+		  event->arg = list3 (make_pointer_integer (ns->desc),
+				      action, fname);
+		  kbd_buffer_store_event (event);
+		  (*evcount)++;
+		  if (!fni->NextEntryOffset)
+		    break;
+		  p += fni->NextEntryOffset;
+		  fni = (PFILE_NOTIFY_INFORMATION)p;
+		  info_size -= fni->NextEntryOffset;
+		}
 	    }
+	  /* Free this notifications set. */
+	  xfree (ns->notifications);
+	  xfree (ns);
 	}
-      notification_buffer_in_use = 0;
     }
-  else
-    DebPrint (("We were promised notifications, but in-use flag is zero!\n"));
-  leave_crit ();
-
   /* We've stuffed all the events ourselves, so w32_read_socket shouldn't.  */
   event->kind = NO_EVENT;
 }
@@ -6949,6 +6963,8 @@ w32_init_main_thread (void)
   DuplicateHandle (GetCurrentProcess (), GetCurrentThread (),
 		   GetCurrentProcess (), &hMainThread, 0, TRUE,
 		   DUPLICATE_SAME_ACCESS);
+
+
 }
 
 DWORD WINAPI w32_msg_worker (void * arg);
diff --git a/src/w32term.h b/src/w32term.h
index d809be3c03..8585c8190d 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -727,10 +727,18 @@ extern void x_delete_display (struct w32_display_info *dpyinfo);
 
 extern void x_query_color (struct frame *, XColor *);
 
-extern volatile int notification_buffer_in_use;
-extern BYTE file_notifications[16384];
-extern DWORD notifications_size;
-extern void *notifications_desc;
+#define FILE_NOTIFICATIONS_SIZE 16384
+/* Notifications come in sets.  We use a doubly linked list with a
+   sentinel to communicate those sets from the watching threads to the
+   main thread.  */
+struct notifications_set {
+  LPBYTE notifications;
+  DWORD size;
+  void *desc;
+  struct notifications_set *next;
+  struct notifications_set *prev;
+};
+extern struct notifications_set *notifications_set_head;
 extern Lisp_Object w32_get_watch_object (void *);
 extern Lisp_Object lispy_file_action (DWORD);
 extern int handle_file_notifications (struct input_event *);
diff --git a/src/w32xfns.c b/src/w32xfns.c
index 04bf5ce733..9b633c4c56 100644
--- a/src/w32xfns.c
+++ b/src/w32xfns.c
@@ -48,6 +48,19 @@ init_crit (void)
      when the input queue is empty, so make it a manual reset event. */
   input_available = CreateEvent (NULL, TRUE, FALSE, NULL);
 
+  /* Initialize the linked list of notifications sets that will be
+     used to communicate between the watching worker threads and the
+     main thread.  */
+  notifications_set_head = malloc (sizeof(struct notifications_set));
+  if (notifications_set_head)
+    {
+      memset (notifications_set_head, 0, sizeof(struct notifications_set));
+      notifications_set_head->next
+	= notifications_set_head->prev = notifications_set_head;
+    }
+  else
+    DebPrint(("Out of memory: can't initialize notifications sets."));
+
 #ifdef WINDOWSNT
   keyboard_handle = input_available;
 #endif /* WINDOWSNT */
@@ -76,6 +89,21 @@ delete_crit (void)
       CloseHandle (interrupt_handle);
       interrupt_handle = NULL;
     }
+
+  if (notifications_set_head)
+    {
+      /* Free any remaining notifications set that could be left over.  */
+      while (notifications_set_head->next != notifications_set_head)
+	{
+	  struct notifications_set *ns = notifications_set_head->next;
+	  notifications_set_head->next = ns->next;
+	  ns->next->prev = notifications_set_head;
+	  if (ns->notifications)
+	    free (ns->notifications);
+	  free (ns);
+	}
+    }
+  free (notifications_set_head);
 }
 
 void
diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el
index d3610f0008..39088949b3 100644
--- a/test/lisp/filenotify-tests.el
+++ b/test/lisp/filenotify-tests.el
@@ -433,7 +433,8 @@ longer than timeout seconds for the events to be delivered."
 	    (write-region
 	     "any text" nil file-notify--test-tmpfile nil 'no-message)
 	    (read-event nil nil file-notify--test-read-event-timeout)
-	    (delete-directory temporary-file-directory 'recursive))
+            (delete-directory temporary-file-directory 'recursive)
+            (read-event nil nil file-notify--test-read-event-timeout))
           (file-notify-rm-watch file-notify--test-desc))
 
         ;; Check copy of files inside a directory.
@@ -475,7 +476,8 @@ longer than timeout seconds for the events to be delivered."
 	    (read-event nil nil file-notify--test-read-event-timeout)
 	    (set-file-times file-notify--test-tmpfile '(0 0))
 	    (read-event nil nil file-notify--test-read-event-timeout)
-	    (delete-directory temporary-file-directory 'recursive))
+            (delete-directory temporary-file-directory 'recursive)
+            (read-event nil nil file-notify--test-read-event-timeout))
           (file-notify-rm-watch file-notify--test-desc))
 
         ;; Check rename of files inside a directory.
@@ -509,7 +511,8 @@ longer than timeout seconds for the events to be delivered."
 	    (rename-file file-notify--test-tmpfile file-notify--test-tmpfile1)
 	    ;; After the rename, we won't get events anymore.
 	    (read-event nil nil file-notify--test-read-event-timeout)
-	    (delete-directory temporary-file-directory 'recursive))
+            (delete-directory temporary-file-directory 'recursive)
+            (read-event nil nil file-notify--test-read-event-timeout))
           (file-notify-rm-watch file-notify--test-desc))
 
         ;; Check attribute change.  Does not work for cygwin.
@@ -527,7 +530,7 @@ longer than timeout seconds for the events to be delivered."
 	       ;; w32notify does not distinguish between `changed' and
 	       ;; `attribute-changed'.
 	       ((string-equal (file-notify--test-library) "w32notify")
-		'(changed changed changed changed))
+		'(changed changed))
 	       ;; For kqueue and in the remote case, `write-region'
 	       ;; raises also an `attribute-changed' event.
 	       ((or (string-equal (file-notify--test-library) "kqueue")
@@ -754,7 +757,9 @@ longer than timeout seconds for the events to be delivered."
         (should (file-notify-valid-p file-notify--test-desc))
         ;; After removing the watch, the descriptor must not be valid
         ;; anymore.
+        (read-event nil nil file-notify--test-read-event-timeout)
         (file-notify-rm-watch file-notify--test-desc)
+        (read-event nil nil file-notify--test-read-event-timeout)
         (file-notify--wait-for-events
          (file-notify--test-timeout)
 	 (not (file-notify-valid-p file-notify--test-desc)))
@@ -776,7 +781,9 @@ longer than timeout seconds for the events to be delivered."
         (should (file-notify-valid-p file-notify--test-desc))
         ;; After deleting the directory, the descriptor must not be
         ;; valid anymore.
+        (read-event nil nil file-notify--test-read-event-timeout)
         (delete-directory file-notify--test-tmpfile t)
+        (read-event nil nil file-notify--test-read-event-timeout)
         (file-notify--wait-for-events
 	 (file-notify--test-timeout)
 	 (not (file-notify-valid-p file-notify--test-desc)))