Subject: bug#22534: File notify broken on Windows

From: Eli Zaretskii <[email-masked]>
Date: Sat, 06 Feb 2016 18:53:04 +0200

> Date: Fri, 05 Feb 2016 21:43:26 +0200
> From: Eli Zaretskii <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> > From: Michael Albinus <[email-masked]>
> > Cc: Eli Zaretskii <[email-masked]>,  [email-masked]
> > Date: Fri, 05 Feb 2016 18:18:59 +0100
> > 
> > > I have tried with different values of file-notify--test-timeout but it
> > > doesn't seem to make any difference. I also boosted the size of the
> > > file_notifications[] array (* 16) without any difference. I have
> > > tested with sit-for between each rename operation and it does not work
> > > either above 260 files
> > 
> > Is it worth to continue to look for the problem in w32notify? I would simply
> > set the loop limit there to 250, that's it.
> 
> It would be a waste of your time to look into that.  Go ahead and
> lower the number; I will look into the problem when I have time and
> see what come up with.  We can then pick up where we left off.

I've looked into this.  The simple patch below fixes the test for me.

The problem was that all the 1000 renames were run in a single long
series.  On MS-Windows this generates 2000 events, because the target
file exists and needs to be deleted first.  However, w32notify has a
static limit of about 1000 events it can collect without losing some
(that's on a 32-bit system, probably less on a 64-bit OS).  The last
part of the riddle is that in batch mode file notifications are not
read on MS-Windows unless someone calls read-event or a similar
function that calls 'pselect' to check for input.  If they are not
read, they keep accumulating, and are then reported all together.

I also suspect that the Windows API use by w32notify has its own limit
of events it can accumulate, and beyond that limit simply drops the
older events on the floor.

The solution is to call read-event while renaming the files.  Then I
could go back to 1000 renames without failures.

Michael, is there a reason not to issue those additional calls to
read-event on other platforms?  If necessary, those calls could be
made conditional on MS-Windows.

Here's the proposed patch.  Fabrice, please see if this works for you
(the full test runs for about 7 minutes, so be patient).  Btw, we
could make the test run faster by lowering the 0.1 argument to the 2
read-event calls while we create the 2000 files at the beginning of
the test, I see no need for waiting that long for a single file
creation.

diff --git a/test/automated/file-notify-tests.el b/test/automated/file-notify-tests.el
index 629d85b..5fc4ff8 100644
--- a/test/automated/file-notify-tests.el
+++ b/test/automated/file-notify-tests.el
@@ -66,7 +66,7 @@ file-notify--test-timeout
   "Timeout to wait for arriving events, in seconds."
   (cond
    ((file-remote-p temporary-file-directory) 6)
-   ((string-equal (file-notify--test-library) "w32notify") 20)
+   ((string-equal (file-notify--test-library) "w32notify") 10)
    ((eq system-type 'cygwin) 10)
    (t 3)))
 
@@ -797,10 +797,7 @@ file-notify--test-with-events
 	  file-notify--test-tmpfile
 	  '(change) 'file-notify--test-event-handler)))
   (unwind-protect
-      ;; In case of w32notify, the upper limit of events to handle
-      ;; seems to be 260.  Reason unknown.
-      (let ((n (if (string-equal (file-notify--test-library) "w32notify")
-                   250 1000))
+      (let ((n 1000)
             source-file-list target-file-list
             (default-directory file-notify--test-tmpfile))
         (dotimes (i n)
@@ -832,10 +829,11 @@ file-notify--test-with-events
           (let ((source-file-list source-file-list)
                 (target-file-list target-file-list))
             (while (and source-file-list target-file-list)
-              (rename-file (pop source-file-list) (pop target-file-list) t))))
+              (rename-file (pop source-file-list) (pop target-file-list) t)
+              (read-event nil nil 0.02))))
         (file-notify--test-with-events (make-list n 'deleted)
           (dolist (file target-file-list)
-            (delete-file file))))
+            (prog1 (delete-file file) (read-event nil nil 0.02)))))
     (file-notify--test-cleanup)))
 
 (file-notify--deftest-remote file-notify-test06-many-events