[Emacs-diffs] master 4849677: Fix Bug#22814

[Michael Albinus]

branch: master
commit 484967796755051c4045cdcc26b0d3d129cc72ad
Author: Michael Albinus <address@hidden>
Commit: Michael Albinus <address@hidden>

    Fix Bug#22814
    
    * lisp/autorevert.el (global-auto-revert-mode): Do not set
    `auto-revert-use-notify' to nil.
    
    * etc/NEWS: Mention this.
    
    * etc/PROBLEMS: Remove problem Bug#22814.
    
    * src/kqueue.c: Include <sys/resource.h>.
    (Fkqueue_add_watch): Limit the number of used file descriptors.
    (Bug#22814)
    
    * test/lisp/filenotify-tests.el (file-notify--test-remote-enabled)
    (file-notify-test00-availability, file-notify-test01-add-watch)
    (file-notify-test02-events, file-notify-test06-many-events):
    Use #' read syntax for functions.
    (file-notify-test05-dir-validity)
    (file-notify-test06-many-events): Simplify directory creation.
    (file-notify-test09-sufficient-ressources): New test.
---
 etc/NEWS                      |    3 +
 etc/PROBLEMS                  |   11 -----
 lisp/autorevert.el            |    6 +--
 src/kqueue.c                  |   24 +++++++++++-
 test/lisp/filenotify-tests.el |   86 ++++++++++++++++++++++++++++++----------
 5 files changed, 91 insertions(+), 39 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1725776..d7e1b83 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -114,6 +114,9 @@ different group ID.
 +++
 *** New connection method "doas" for OpenBSD hosts.
 
+---
+** `auto-revert-use-notify' is set back to t in `global-auto-revert-mode'.
+
 
 * New Modes and Packages in Emacs 25.2
 
diff --git a/etc/PROBLEMS b/etc/PROBLEMS
index d531367..7d78037 100644
--- a/etc/PROBLEMS
+++ b/etc/PROBLEMS
@@ -600,17 +600,6 @@ you have a personal configuration file (normally 
~/.aspell.conf), it
 can cause this error.  Remove that file, execute 'ispell-kill-ispell'
 in Emacs, and then try spell-checking again.
 
-*** Emacs eats all file descriptors when using kqueue file notifications.
-See <http://debbugs.gnu.org/22814>.
-
-When you have a large number of buffers running auto-revert-mode, and
-Emacs is configured to use the kqueue file notification library, it
-uses an own file descriptor for every watched file.  On systems with a
-small limit of file descriptors allowed per process, like OS X, you
-could run out of file descriptors.  You won't be able to open new files.
-
-auto-revert-use-notify is set to nil in global-auto-revert-mode, therefore.
-
 * Runtime problems related to font handling
 
 ** Characters are displayed as empty boxes or with wrong font under X.
diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index bde8eb8..14e39bd 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -458,11 +458,7 @@ specifies in the mode line."
   :global t :group 'auto-revert :lighter global-auto-revert-mode-text
   (auto-revert-set-timer)
   (if global-auto-revert-mode
-      (progn
-        ;; We disable file notification because it could use too many
-        ;; ressources.  See <http://debbugs.gnu.org/22814>.
-        (setq auto-revert-use-notify nil)
-        (auto-revert-buffers))
+      (auto-revert-buffers)
     (dolist (buf (buffer-list))
       (with-current-buffer buf
        (when auto-revert-use-notify
diff --git a/src/kqueue.c b/src/kqueue.c
index a69d06d..7e3bfdd 100644
--- a/src/kqueue.c
+++ b/src/kqueue.c
@@ -29,6 +29,10 @@ along with GNU Emacs.  If not, see 
<http://www.gnu.org/licenses/>.  */
 #include "keyboard.h"
 #include "process.h"
 
+#ifdef HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif /* HAVE_SYS_RESOURCE_H  */
+
 
 /* File handle for kqueue.  */
 static int kqueuefd = -1;
@@ -368,9 +372,12 @@ only when the upper directory of the renamed file is 
watched.  */)
   (Lisp_Object file, Lisp_Object flags, Lisp_Object callback)
 {
   Lisp_Object watch_object, dir_list;
-  int fd, oflags;
+  int maxfd, fd, oflags;
   u_short fflags = 0;
   struct kevent kev;
+#ifdef HAVE_GETRLIMIT
+  struct rlimit rlim;
+#endif /* HAVE_GETRLIMIT  */
 
   /* Check parameters.  */
   CHECK_STRING (file);
@@ -383,6 +390,21 @@ only when the upper directory of the renamed file is 
watched.  */)
   if (! FUNCTIONP (callback))
     wrong_type_argument (Qinvalid_function, callback);
 
+  /* Check available file descriptors.  */
+#ifdef HAVE_GETRLIMIT
+  if (! getrlimit (RLIMIT_NOFILE, &rlim))
+    maxfd = rlim.rlim_cur;
+  else
+#endif /* HAVE_GETRLIMIT  */
+    maxfd = 256;
+
+  /* We assume 50 file descriptors are sufficient for the rest of Emacs.  */
+  if ((maxfd - 50) < XINT (Flength (watch_list)))
+    xsignal2
+      (Qfile_notify_error,
+       build_string ("File watching not possible, no file descriptor left"),
+       Flength (watch_list));
+
   if (kqueuefd < 0)
     {
       /* Create kqueue descriptor.  */
diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el
index 9f0c0ed..d3610f0 100644
--- a/test/lisp/filenotify-tests.el
+++ b/test/lisp/filenotify-tests.el
@@ -140,7 +140,7 @@ being the result.")
          (setq desc
                (file-notify-add-watch
                 file-notify-test-remote-temporary-file-directory
-                '(change) 'ignore))))
+                '(change) #'ignore))))
       (setq file-notify--test-remote-enabled-checked (cons t desc))
       (when desc (file-notify-rm-watch desc))))
   ;; Return result.
@@ -180,7 +180,7 @@ remote host, or nil."
   (message "Library: `%s'" (file-notify--test-library))
   (should
    (setq file-notify--test-desc
-         (file-notify-add-watch temporary-file-directory '(change) 'ignore)))
+         (file-notify-add-watch temporary-file-directory '(change) #'ignore)))
 
   ;; Cleanup.
   (file-notify--test-cleanup))
@@ -199,23 +199,23 @@ remote host, or nil."
   ;; Check, that different valid parameters are accepted.
   (should
    (setq file-notify--test-desc
-         (file-notify-add-watch temporary-file-directory '(change) 'ignore)))
+         (file-notify-add-watch temporary-file-directory '(change) #'ignore)))
   (file-notify-rm-watch file-notify--test-desc)
   (should
    (setq file-notify--test-desc
          (file-notify-add-watch
-          temporary-file-directory '(attribute-change) 'ignore)))
+          temporary-file-directory '(attribute-change) #'ignore)))
   (file-notify-rm-watch file-notify--test-desc)
   (should
    (setq file-notify--test-desc
          (file-notify-add-watch
-          temporary-file-directory '(change attribute-change) 'ignore)))
+          temporary-file-directory '(change attribute-change) #'ignore)))
   (file-notify-rm-watch file-notify--test-desc)
   (write-region "any text" nil file-notify--test-tmpfile nil 'no-message)
   (should
    (setq file-notify--test-desc
          (file-notify-add-watch
-          file-notify--test-tmpfile '(change attribute-change) 'ignore)))
+          file-notify--test-tmpfile '(change attribute-change) #'ignore)))
   (file-notify-rm-watch file-notify--test-desc)
   (delete-file file-notify--test-tmpfile)
 
@@ -238,7 +238,7 @@ remote host, or nil."
   (should
    (equal (should-error
            (file-notify-add-watch
-            file-notify--test-tmpfile1 '(change attribute-change) 'ignore))
+            file-notify--test-tmpfile1 '(change attribute-change) #'ignore))
           `(file-notify-error
             "Directory does not exist" ,file-notify--test-tmpfile)))
 
@@ -361,7 +361,7 @@ longer than timeout seconds for the events to be delivered."
            (setq file-notify--test-desc
                  (file-notify-add-watch
                   file-notify--test-tmpfile
-                  '(change) 'file-notify--test-event-handler)))
+                  '(change) #'file-notify--test-event-handler)))
           (file-notify--test-with-events
               (cond
                ;; cygwin recognizes only `deleted' and `stopped' events.
@@ -381,7 +381,7 @@ longer than timeout seconds for the events to be delivered."
         (setq file-notify--test-desc
               (file-notify-add-watch
                file-notify--test-tmpfile
-               '(change) 'file-notify--test-event-handler)))
+               '(change) #'file-notify--test-event-handler)))
         (file-notify--test-with-events
            (cond
             ;; cygwin recognizes only `deleted' and `stopped' events.
@@ -414,7 +414,7 @@ longer than timeout seconds for the events to be delivered."
                 file-notify--test-desc
                 (file-notify-add-watch
                  temporary-file-directory
-                 '(change) 'file-notify--test-event-handler)))
+                 '(change) #'file-notify--test-event-handler)))
          (file-notify--test-with-events
              (cond
               ;; w32notify does not raise `deleted' and `stopped'
@@ -445,7 +445,7 @@ longer than timeout seconds for the events to be delivered."
                 file-notify--test-desc
                 (file-notify-add-watch
                  temporary-file-directory
-                 '(change) 'file-notify--test-event-handler)))
+                 '(change) #'file-notify--test-event-handler)))
          (file-notify--test-with-events
              (cond
               ;; w32notify does not distinguish between `changed' and
@@ -487,7 +487,7 @@ longer than timeout seconds for the events to be delivered."
                 file-notify--test-desc
                 (file-notify-add-watch
                  temporary-file-directory
-                 '(change) 'file-notify--test-event-handler)))
+                 '(change) #'file-notify--test-event-handler)))
          (file-notify--test-with-events
              (cond
               ;; w32notify does not raise `deleted' and `stopped'
@@ -521,7 +521,7 @@ longer than timeout seconds for the events to be delivered."
           (setq file-notify--test-desc
                 (file-notify-add-watch
                  file-notify--test-tmpfile
-                 '(attribute-change) 'file-notify--test-event-handler)))
+                 '(attribute-change) #'file-notify--test-event-handler)))
          (file-notify--test-with-events
              (cond
               ;; w32notify does not distinguish between `changed' and
@@ -743,9 +743,9 @@ longer than timeout seconds for the events to be delivered."
 
   (unwind-protect
       (progn
-        (setq file-notify--test-tmpfile
-             (file-name-as-directory (file-notify--test-make-temp-name)))
-        (make-directory file-notify--test-tmpfile)
+       (should
+        (setq file-notify--test-tmpfile
+              (make-temp-file "file-notify-test-parent" t)))
        (should
         (setq file-notify--test-desc
               (file-notify-add-watch
@@ -765,9 +765,9 @@ longer than timeout seconds for the events to be delivered."
 
   (unwind-protect
       (progn
-       (setq file-notify--test-tmpfile
-             (file-name-as-directory (file-notify--test-make-temp-name)))
-        (make-directory file-notify--test-tmpfile)
+       (should
+        (setq file-notify--test-tmpfile
+              (make-temp-file "file-notify-test-parent" t)))
        (should
         (setq file-notify--test-desc
               (file-notify-add-watch
@@ -795,13 +795,14 @@ longer than timeout seconds for the events to be 
delivered."
   ;; Under cygwin events arrive in random order.  Impossible to define a test.
   (skip-unless (not (eq system-type 'cygwin)))
 
-  (setq file-notify--test-tmpfile (file-notify--test-make-temp-name))
-  (make-directory file-notify--test-tmpfile)
+  (should
+   (setq file-notify--test-tmpfile
+        (make-temp-file "file-notify-test-parent" t)))
   (should
    (setq file-notify--test-desc
         (file-notify-add-watch
          file-notify--test-tmpfile
-         '(change) 'file-notify--test-event-handler)))
+         '(change) #'file-notify--test-event-handler)))
   (unwind-protect
       (let ((n 1000)
             source-file-list target-file-list
@@ -1058,6 +1059,47 @@ the file watch."
 (file-notify--deftest-remote file-notify-test08-watched-file-in-watched-dir
   "Check `file-notify-test08-watched-file-in-watched-dir' for remote files.")
 
+(ert-deftest file-notify-test09-sufficient-ressources ()
+  "Check that file notification does not use too many ressources."
+  :tags '(:expensive-test)
+  (skip-unless (file-notify--test-local-enabled))
+  ;; This test is intended for kqueue only.
+  (skip-unless (string-equal (file-notify--test-library) "kqueue"))
+
+  (should
+   (setq file-notify--test-tmpfile
+        (make-temp-file "file-notify-test-parent" t)))
+  (unwind-protect
+      (let ((temporary-file-directory file-notify--test-tmpfile)
+           descs)
+       (should-error
+        (while t
+          ;; We watch directories, because we want to reach the upper
+          ;; limit.  Watching a file might not be sufficient, because
+          ;; most of the libraries implement this as watching the
+          ;; upper directory.
+          (setq file-notify--test-tmpfile1
+                (make-temp-file "file-notify-test-parent" t)
+                descs
+                (cons
+                 (should
+                  (file-notify-add-watch
+                   file-notify--test-tmpfile1 '(change) #'ignore))
+                 descs)))
+        :type 'file-notify-error)
+       ;; Remove watches.  If we don't do it prior removing
+       ;; directories, Emacs crashes in batch mode.
+       (dolist (desc descs)
+        (file-notify-rm-watch desc))
+       ;; Remove directories.
+        (delete-directory file-notify--test-tmpfile 'recursive))
+
+    ;; Cleanup.
+    (file-notify--test-cleanup)))
+
+(file-notify--deftest-remote file-notify-test09-sufficient-ressources
+  "Check `file-notify-test09-sufficient-ressources' for remote files.")
+
 (defun file-notify-test-all (&optional interactive)
   "Run all tests for \\[file-notify]."
   (interactive "p")