bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

1. Copy stp-test.el and test.sh to the same directory.
2. Evaluate the .el file.
3. M-x global-auto-revert-mode.
4. M-x start-file-process-test.
5. See the message log. "sleeping done!" comes before "process done!".

Without step 3, "process done!" comes before "sleeping done!".

So it looks like, effectively, global-auto-revert-mode stops sleep-for
from calling process sentinels, at least.

I don't remember seeing anything like this in Emacs 25.

sit-for is not affected, BTW.

In GNU Emacs 26.0.50 (build 10, x86_64-pc-linux-gnu, GTK+ Version 3.20.9)
  of 2017-05-14 built on zappa
Repository revision: e6f64df9c2b443d3385c2c25c29ccd5283d37e3f
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 16.10

[-- Attachment #2: sfp-test.el --]
[-- Type: text/x-emacs-lisp, Size: 408 bytes --]

(defun start-file-process-test ()
  (interactive)
  (let* ((buf (get-buffer-create "*test*"))
         (process (start-file-process "foo" buf
                                      (executable-find "sh")
                                      "./test.sh")))
    (set-process-sentinel
     process
     (lambda (proc status)
       (message "process done!")))
    (sleep-for 3)
    (message "sleeping done!")))

[-- Attachment #3: test.sh --]
[-- Type: application/x-shellscript, Size: 22 bytes --]

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 18 May 2017 02:57:17 +0300
> 
> 1. Copy stp-test.el and test.sh to the same directory.
> 2. Evaluate the .el file.
> 3. M-x global-auto-revert-mode.
> 4. M-x start-file-process-test.
> 5. See the message log. "sleeping done!" comes before "process done!".
> 
> Without step 3, "process done!" comes before "sleeping done!".
> 
> So it looks like, effectively, global-auto-revert-mode stops sleep-for
> from calling process sentinels, at least.
> 
> I don't remember seeing anything like this in Emacs 25.

Does anything change if you set auto-revert-use-notify to a nil value?

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Dmitry Gutov]

On 20.05.2017 14:36, Eli Zaretskii wrote:

> Does anything change if you set auto-revert-use-notify to a nil value?

Yes: that seems to fix the problem.

In anticipation of the next question, my value of file-notify--library 
is `inotify'.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> Cc: 26973@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 20 May 2017 15:45:57 +0300
> 
> On 20.05.2017 14:36, Eli Zaretskii wrote:
> 
> > Does anything change if you set auto-revert-use-notify to a nil value?
> 
> Yes: that seems to fix the problem.
> 
> In anticipation of the next question, my value of file-notify--library 
> is `inotify'.

Right, that's what I thought.  I'm sure Michael (CC'ed) will look into
this.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Dmitry Gutov]

On 20.05.2017 16:38, Eli Zaretskii wrote:

>> In anticipation of the next question, my value of file-notify--library
>> is `inotify'.
> 
> Right, that's what I thought.  I'm sure Michael (CC'ed) will look into
> this.

Thanks, waiting for Michael.

BTW, accept-process-output also seems broken in similar scenarios.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> Cc: 26973@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 22 May 2017 01:38:31 +0300
> 
> BTW, accept-process-output also seems broken in similar scenarios.

Of course: both accept-process-output and sleep-for call the same
low-level API.

I hope it isn't something related to threads...

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Dmitry Gutov]

On 5/22/17 7:06 AM, Eli Zaretskii wrote:

>> BTW, accept-process-output also seems broken in similar scenarios.
> 
> Of course: both accept-process-output and sleep-for call the same
> low-level API.

Does sit-for call the same API?

Because replacing (sleep-for 3) with (sit-for 3), or even (sit-for 3 t), 
avoids triggering the problem described here.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> Cc: michael.albinus@gmx.de, 26973@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 May 2017 03:58:01 +0300
> 
> On 5/22/17 7:06 AM, Eli Zaretskii wrote:
> 
> >> BTW, accept-process-output also seems broken in similar scenarios.
> > 
> > Of course: both accept-process-output and sleep-for call the same
> > low-level API.
> 
> Does sit-for call the same API?

It calls it in a slightly different manner.

> Because replacing (sleep-for 3) with (sit-for 3), or even (sit-for 3 t), 
> avoids triggering the problem described here.

Yes, I understand.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Does anything change if you set auto-revert-use-notify to a nil value?
>> 
>> Yes: that seems to fix the problem.
>> 
>> In anticipation of the next question, my value of file-notify--library 
>> is `inotify'.
>
> Right, that's what I thought.  I'm sure Michael (CC'ed) will look into
> this.

Will do. But this might take time; sleep-for and accept-process-output
are terra incognita for me until now.

Best regards, Michael.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  26973@debbugs.gnu.org
> Date: Mon, 22 May 2017 09:52:45 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> > Does anything change if you set auto-revert-use-notify to a nil value?
> >> 
> >> Yes: that seems to fix the problem.
> >> 
> >> In anticipation of the next question, my value of file-notify--library 
> >> is `inotify'.
> >
> > Right, that's what I thought.  I'm sure Michael (CC'ed) will look into
> > this.
> 
> Will do. But this might take time; sleep-for and accept-process-output
> are terra incognita for me until now.

Let us know if you need any help.

Thanks.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> Will do. But this might take time; sleep-for and accept-process-output
>> are terra incognita for me until now.
>
> Let us know if you need any help.

I've tried to bisect the master branch in order to see when the problem
started. Kind of impossible, due to the heavy changes in configuration
machinery of Emacs last weeks.

So I've followed the commits for inotify.c. With commit b2a83eed23 the
problem doesn't happen. It started to appear with next commit for
inotify.c, 158bb8555d.

I've Cc'ed Andreas, who's the author of that commit.

> Thanks.

Best regards, Michael.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]


I seems that the event flags used are to promiscuous and that this
prevents other processes from reading.  Removing ACCESS, OPEN and
CLOSE events appears to rectify the situation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff, Size: 1344 bytes --]

diff --git a/src/inotify.c b/src/inotify.c
index 290701349e..d43b959747 100644
--- a/src/inotify.c
+++ b/src/inotify.c
@@ -41,7 +41,21 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #ifndef IN_ONLYDIR
 # define IN_ONLYDIR 0
 #endif
-#define INOTIFY_DEFAULT_MASK (IN_ALL_EVENTS | IN_EXCL_UNLINK)
+#define INOTIFY_DEFAULT_MASK                                    \
+  (IN_ATTRIB |                                                  \
+   /* IN_ACCESS | */                                            \
+   /* IN_CLOSE_WRITE | */                                       \
+   /* IN_CLOSE_NOWRITE |  */                                    \
+   IN_CREATE |                                                  \
+   IN_DELETE |                                                  \
+   IN_DELETE_SELF |                                             \
+   IN_IGNORED |                                                 \
+   IN_MODIFY |                                                  \
+   IN_MOVE_SELF |                                               \
+   IN_MOVED_FROM |                                              \
+   IN_MOVED_TO |                                                \
+   /* IN_OPEN | */                                              \
+   IN_EXCL_UNLINK)
 
 /* File handle for inotify.  */
 static int inotifyfd = -1;

[-- Attachment #3: Type: text/plain, Size: 5 bytes --]


-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Michael Albinus]

Andreas Politz <politza@hochschule-trier.de> writes:

Hi Andreas,

> I seems that the event flags used are to promiscuous and that this
> prevents other processes from reading.  Removing ACCESS, OPEN and
> CLOSE events appears to rectify the situation.

The patch ought to fix the bug, thanks. filenotify-tests.el pass also
all tests.

So I have committed the patch in your name. Dmitry, could you pls crosscheck?

> -ap

Best regards, Michael.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Dmitry Gutov]

On 5/26/17 5:45 PM, Michael Albinus wrote:

> The patch ought to fix the bug, thanks. filenotify-tests.el pass also
> all tests.
> 
> So I have committed the patch in your name. Dmitry, could you pls crosscheck?

Looks fixed to me as well, thank you.

But I'm also looking forward to your response to Paul's concerns

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

> But I'm also looking forward to your response to Paul's concerns

I let this for Andreas.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

> the event flags used are to[o] promiscuous and that this
> prevents other processes from reading.  Removing ACCESS, OPEN and
> CLOSE events appears to rectify the situation.

This means that a later call to inotify-add-watch that lacks IN_OPEN 
will disable an already-existing watch that specifies IN_OPEN, right? 
That doesn't sound right.

I reviewed the inotify.c patches made since March and spotted what 
appear to be some problems related to this. What do you think of the 
attached patches? The first patch doesn't change behavior; it merely 
makes the later patches easier to write. The second patch restores 
onlydir (it appears to have been removed by mistake). The third patch 
uses IN_MASK_ADD instead of a promiscuous mask.


[-- Attachment #2: 0001-Simplify-computation-of-inotify-mask.patch --]
[-- Type: text/x-patch, Size: 2144 bytes --]

From 46eb61f91160f51e2c16c14b327a2d2f8145d186 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 26 May 2017 09:37:44 -0700
Subject: [PATCH 1/3] Simplify computation of inotify mask

* src/inotify.c (add_watch): Accept uint32_t imask instead
of Lisp_Object aspect.  Caller changed.
(Finotify_add_watch): Use aspect_to_inotifymask earlier, to
simplify the code.
---
 src/inotify.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/inotify.c b/src/inotify.c
index 1165293..bcf30ad 100644
--- a/src/inotify.c
+++ b/src/inotify.c
@@ -215,16 +215,15 @@ inotifyevent_to_event (Lisp_Object watch, struct inotify_event const *ev)
 }
 
 /* Add a new watch to watch-descriptor WD watching FILENAME and using
-   CALLBACK.  Returns a cons (DESCRIPTOR . ID) uniquely identifying the
-   new watch.  */
+   IMASK and CALLBACK.  Return a cons (DESCRIPTOR . ID) uniquely
+   identifying the new watch.  */
 static Lisp_Object
 add_watch (int wd, Lisp_Object filename,
-	   Lisp_Object aspect, Lisp_Object callback)
+	   uint32_t imask, Lisp_Object callback)
 {
   Lisp_Object descriptor = INTEGER_TO_CONS (wd);
   Lisp_Object tail = assoc_no_quit (descriptor, watch_list);
   Lisp_Object watch, watch_id;
-  uint32_t imask = aspect_to_inotifymask (aspect);
   Lisp_Object mask = INTEGER_TO_CONS (imask);
 
   EMACS_INT id = 0;
@@ -436,12 +435,10 @@ IN_ONLYDIR  */)
      (Lisp_Object filename, Lisp_Object aspect, Lisp_Object callback)
 {
   Lisp_Object encoded_file_name;
-  bool dont_follow = (CONSP (aspect)
-		      ? ! NILP (Fmemq (Qdont_follow, aspect))
-		      : EQ (Qdont_follow, aspect));
   int wd = -1;
+  uint32_t imask = aspect_to_inotifymask (aspect);
   uint32_t mask = (INOTIFY_DEFAULT_MASK
-                   | (dont_follow ? IN_DONT_FOLLOW : 0));
+		   | (imask & IN_DONT_FOLLOW));
 
   CHECK_STRING (filename);
 
@@ -459,7 +456,7 @@ IN_ONLYDIR  */)
   if (wd < 0)
     report_file_notify_error ("Could not add watch for file", filename);
 
-  return add_watch (wd, filename, aspect, callback);
+  return add_watch (wd, filename, imask, callback);
 }
 
 static bool
-- 
2.9.4


[-- Attachment #3: 0002-Restore-inotify-onlydir-support.patch --]
[-- Type: text/x-patch, Size: 2465 bytes --]

From d8ada0da7041e524b97c4354facd8803716d92bf Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 26 May 2017 09:50:56 -0700
Subject: [PATCH 2/3] Restore inotify onlydir support

There was no need to remove it in the 2017-03-26 inotify change,
as it is like IN_DONT_FOLLOW and does not affect other watchers
for the same file.
* src/inotify.c (symbol_to_inotifymask, Finotify_add_watch)
(syms_of_inotify): Bring back onlydir.
---
 src/inotify.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/inotify.c b/src/inotify.c
index bcf30ad..b3e0728 100644
--- a/src/inotify.c
+++ b/src/inotify.c
@@ -72,10 +72,6 @@ static int inotifyfd = -1;
    IN_EXCL_UNLINK
    IN_MASK_ADD
    IN_ONESHOT
-   IN_ONLYDIR
-
-   FIXME: Explain why IN_ONLYDIR is in the list, as it seems to be
-   in the same category as IN_DONT_FOLLOW which is allowed.
 
    Each element of this list is of the form (DESCRIPTOR . WATCHES)
    where no two DESCRIPTOR values are the same.  DESCRIPTOR represents
@@ -162,6 +158,8 @@ symbol_to_inotifymask (Lisp_Object symb)
 
   else if (EQ (symb, Qdont_follow))
     return IN_DONT_FOLLOW;
+  else if (EQ (symb, Qonlydir))
+    return IN_ONLYDIR;
 
   else if (EQ (symb, Qt) || EQ (symb, Qall_events))
     return IN_ALL_EVENTS;
@@ -397,9 +395,11 @@ all-events or t
 move
 close
 
-The following symbols can also be added to a list of aspects:
+ASPECT can also contain the following symbols, which control whether
+the watch descriptor will be created:
 
 dont-follow
+onlydir
 
 Watching a directory is not recursive.  CALLBACK is passed a single argument
 EVENT which contains an event structure of the format
@@ -430,15 +430,14 @@ shared across different callers.
 
 IN_EXCL_UNLINK
 IN_MASK_ADD
-IN_ONESHOT
-IN_ONLYDIR  */)
+IN_ONESHOT  */)
      (Lisp_Object filename, Lisp_Object aspect, Lisp_Object callback)
 {
   Lisp_Object encoded_file_name;
   int wd = -1;
   uint32_t imask = aspect_to_inotifymask (aspect);
   uint32_t mask = (INOTIFY_DEFAULT_MASK
-		   | (imask & IN_DONT_FOLLOW));
+		   | (imask & (IN_DONT_FOLLOW | IN_ONLYDIR)));
 
   CHECK_STRING (filename);
 
@@ -548,6 +547,7 @@ syms_of_inotify (void)
   DEFSYM (Qclose, "close");		/* IN_CLOSE */
 
   DEFSYM (Qdont_follow, "dont-follow");	/* IN_DONT_FOLLOW */
+  DEFSYM (Qonlydir, "onlydir");		/* IN_ONLYDIR */
 
   DEFSYM (Qignored, "ignored");		/* IN_IGNORED */
   DEFSYM (Qisdir, "isdir");		/* IN_ISDIR */
-- 
2.9.4


[-- Attachment #4: 0003-Depromiscuify-inotify-with-IN_MASK_ADD.patch --]
[-- Type: text/x-patch, Size: 1968 bytes --]

From 0c581863f4b1f3ef31397357dd454fe68416e645 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 26 May 2017 10:53:02 -0700
Subject: [PATCH 3/3] Depromiscuify inotify with IN_MASK_ADD

Use IN_MASK_ADD instead of using a no-longer-promiscuous-enough
mask.  Without this fix, a later watch that omits IN_OPEN disables
an earlier watch that uses IN_OPEN (Bug#26973).
* src/inotify.c (INOTIFY_DEFAULT_MASK): Remove.
(Finotify_add_watch): Use IN_MASK_ADD instead.
---
 src/inotify.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/src/inotify.c b/src/inotify.c
index b3e0728..3d5d3d2 100644
--- a/src/inotify.c
+++ b/src/inotify.c
@@ -42,21 +42,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 # define IN_ONLYDIR 0
 #endif
 
-/* Events that inotify-add-watch waits for.  This list has all the
-   events that any watcher could include, because we want to support
-   multiple watches on the same file even though inotify uses the same
-   descriptor for all watches to that file.  This list omits
-   IN_ACCESS, IN_CLOSE_WRITE, IN_CLOSE_NOWRITE, and IN_OPEN because
-   they would prevent other processes from reading; see Bug#26973.
-
-   FIXME: Explain why it is OK to omit these four bits here even
-   though a inotify-add-watch call might specify them.  */
-
-#define INOTIFY_DEFAULT_MASK                                    \
-  (IN_ATTRIB | IN_CREATE | IN_DELETE | IN_DELETE_SELF		\
-   | IN_IGNORED | IN_MODIFY | IN_MOVE_SELF | IN_MOVED_FROM	\
-   | IN_MOVED_TO | IN_EXCL_UNLINK)
-
 /* File handle for inotify.  */
 static int inotifyfd = -1;
 
@@ -436,8 +421,7 @@ IN_ONESHOT  */)
   Lisp_Object encoded_file_name;
   int wd = -1;
   uint32_t imask = aspect_to_inotifymask (aspect);
-  uint32_t mask = (INOTIFY_DEFAULT_MASK
-		   | (imask & (IN_DONT_FOLLOW | IN_ONLYDIR)));
+  uint32_t mask = imask | IN_MASK_ADD | IN_EXCL_UNLINK;
 
   CHECK_STRING (filename);
 
-- 
2.9.4

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

I think your patches are against an older revision.

Paul Eggert <eggert@cs.ucla.edu> writes:

> [...] a later call to inotify-add-watch that lacks IN_OPEN will
> disable an already-existing watch that specifies IN_OPEN, right? [...]

I don't think so. It means that no one can watch for open/close/access
events, because the flags from the caller are ignored when invoking the
library function (except for IN_DONT_FOLLOW, which in my understanding
does not change the underlying watch).

> [...] The second patch restores onlydir (it appears to have been
> removed by mistake). 

See above, I guess.

> The third patch uses IN_MASK_ADD instead of a promiscuous mask.

We could do that.

-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Paul Eggert]

Andreas Politz wrote:

> I think your patches are against an older revision.

OK, I rebased the patches to master and installed them there (after adjusting to 
the comments below) so you shouldn't have to worry about version mismatch.

>> [...] a later call to inotify-add-watch that lacks IN_OPEN will
>> disable an already-existing watch that specifies IN_OPEN, right? [...]
> 
> I don't think so. It means that no one can watch for open/close/access
> events

Oh, right. I fixed the commit message accordingly.

>> [...] The second patch restores onlydir (it appears to have been
>> removed by mistake).
> 
> See above, I guess.

Yeah, I checked the GNU/Linux source code, and it is a flag that affects only 
the inotify_add_watch call itself, not the watch descriptor, so we don't need to 
worry about it colliding with other watches.

>> The third patch uses IN_MASK_ADD instead of a promiscuous mask.
> 
> We could do that.

Thanks, it's now done as part of the installed patches. I verified that the 
originally-reported bug is still fixed in master, and am marking the bug as done.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

Paul Eggert <eggert@cs.ucla.edu> writes:

>>> [...] The second patch restores onlydir [...].

> Yeah, I checked the GNU/Linux source code, and it is a flag that
> affects only the inotify_add_watch call itself [...]

OK, I haven't been that thorough.  So, I suppose the call will fail for
a combination of onlydir with a non-directory filename.

>>> The third patch uses IN_MASK_ADD instead of a promiscuous mask.

But we still need to inhibit open/close/access events from being used by
any client in order to fix this bug.  (Unless someone finds a better way
by looking more closely into the problem relating to processes. Though
filenotify.el does not use them and so it may not be worth it.)

-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Paul Eggert]

Andreas Politz wrote:
> I suppose the call will fail for
> a combination of onlydir with a non-directory filename.

Yes, that's right.

> we still need to inhibit open/close/access events from being used by
> any client in order to fix this bug.  (Unless someone finds a better way
> by looking more closely into the problem relating to processes. Though
> filenotify.el does not use them and so it may not be worth it.)

I didn't observe the problem with a little test case involving OPEN that I wrote 
myself. I agree it may not be worth looking into it. That being said, if you can 
easily explain the bug or supply a test case I can briefly look into fixing it.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

Paul Eggert <eggert@cs.ucla.edu> writes:

> I didn't observe the problem with a little test case involving OPEN
> that I wrote myself. I agree it may not be worth looking into it. That
> being said, if you can easily explain the bug or supply a test case I
> can briefly look into fixing it.

I didn't really look into it and just assumed that one of those flags
might trigger a circular transition preventing processes from reading.
For that I used the posted function, added a watch to the /tmp directory
and noticed that the problematic behavior occured, if any one of those
flags (open/close/access) were added.

-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Paul Eggert]

Andreas Politz wrote:
> For that I used the posted function, added a watch to the /tmp directory
> and noticed that the problematic behavior occured, if any one of those
> flags (open/close/access) were added.

I couldn't reproduce the problem with the current master (commit 288b3ca). That 
is, I ran src/emacs -Q with this:

M-x load-file RET sfp-test.el RET
M-x global-auto-revert-mode RET
M-x start-file-process-test RET
(inotify-add-watch "/tmp" 'open (lambda (event) (message "event %s" event))) C-x C-e
M-x start-file-process-test RET

Both instances of start-file-process-test did "process done!" before "sleeping 
done!", which I gather is what is wanted.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

Paul Eggert <eggert@cs.ucla.edu> writes:

> I couldn't reproduce the problem with the current master (commit
> 288b3ca). That is, I ran src/emacs -Q with this:

I can't either anymore.

-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

It just occured to me that the other back-ends may exhibit the same
behavior, when used outside of filenotify.el . For example w32notify
provides the flag

  'last-access-time'   -- report changes in last-access time

-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> From: Andreas Politz <politza@hochschule-trier.de>
> Date: Sun, 28 May 2017 11:19:17 +0200
> Cc: 26973-done@debbugs.gnu.org, Michael Albinus <michael.albinus@gmx.de>,
> 	Dmitry Gutov <dgutov@yandex.ru>
> 
> 
> It just occured to me that the other back-ends may exhibit the same
> behavior, when used outside of filenotify.el . For example w32notify
> provides the flag
> 
>   'last-access-time'   -- report changes in last-access time

Sorry, I've lost track of the discussion -- could you please tell what
"same behavior" did you refer to, and what could be the problem with
the last-access-time flag provided by w32notify.c?

Thanks.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Michael Albinus]

Andreas Politz <politza@hochschule-trier.de> writes:

> It just occured to me that the other back-ends may exhibit the same
> behavior, when used outside of filenotify.el . For example w32notify
> provides the flag
>
>   'last-access-time'   -- report changes in last-access time

I've checked the kqueue and gfilenotify cases; the problem does not
occur. I cannot check w32notify.

> -ap

Best regards, Michael.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Sun, 04 Jun 2017 13:49:27 +0200
> Cc: 26973-done@debbugs.gnu.org, Paul Eggert <eggert@cs.ucla.edu>,
> 	Dmitry Gutov <dgutov@yandex.ru>
> 
> Andreas Politz <politza@hochschule-trier.de> writes:
> 
> > It just occured to me that the other back-ends may exhibit the same
> > behavior, when used outside of filenotify.el . For example w32notify
> > provides the flag
> >
> >   'last-access-time'   -- report changes in last-access time
> 
> I've checked the kqueue and gfilenotify cases; the problem does not
> occur. I cannot check w32notify.

Like I said: I've lost track of this discussion.  So could someone
please tell what could be "the problem", and in what situations it
might happen?  Then I could try test that with w32notify.

Thanks.

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Andreas Politz]

Eli Zaretskii <eliz@gnu.org> writes:

> Like I said: I've lost track of this discussion.  [...]

Activating global-auto-revert-mode seems to lead to starvation of the
collection of output from running processes in an inotify build.
Inhibiting the generation of open, close and access file-events seems to
have fixed this problem.

This lead to the hypotheses that, in case the above types of events are
reported, reading file-events while idling may by itself lead to the
generation of more file-events and thus making this process stuck in a
cycle.

Autorevert resp. filenotify.el does not include these events in its
event-mask passed to the various back-ends, such that this problem could
not be perceived, unless the back-ends are used directly.  (Except for
inotify, were a constant, promiscuous mask was previously used in order
to work around a different problem.)

So, if the above is true, other back-ends could exhibit the same
behavior, if they were to be instructed to generate open/close/access
events.  Again, since these events are not used by filenotify.el, it
would only show, if the back-end is invoked directly,
e.g. w32notify-add-watch with last-access-time.

-ap

bug#26973: 26.0.50; sleep-for behavior changes with global-auto-revert-mode enabled

[Eli Zaretskii]

> From: Andreas Politz <politza@hochschule-trier.de>
> Cc: Michael Albinus <michael.albinus@gmx.de>,  26973-done@debbugs.gnu.org,  eggert@cs.ucla.edu,  dgutov@yandex.ru
> Date: Sun, 04 Jun 2017 17:30:15 +0200
> 
> Activating global-auto-revert-mode seems to lead to starvation of the
> collection of output from running processes in an inotify build.
> Inhibiting the generation of open, close and access file-events seems to
> have fixed this problem.

I thought this was inotify-specific, because of the flags used in that
back-end, is that correct?  w32notify uses only the flags specified by
the caller, it doesn't add any flags of its own.

> This lead to the hypotheses that, in case the above types of events are
> reported, reading file-events while idling may by itself lead to the
> generation of more file-events and thus making this process stuck in a
> cycle.

I'm not sure I understand how this could happen.  How can reading
events generate those same events?  Also, is this still related to
invoking sub-processes, or unrelated?

> So, if the above is true, other back-ends could exhibit the same
> behavior, if they were to be instructed to generate open/close/access
> events.  Again, since these events are not used by filenotify.el, it
> would only show, if the back-end is invoked directly,
> e.g. w32notify-add-watch with last-access-time.

So you are saying that if I install a watch for last-access-time
changes, and then change the last-access time of a watched file, I
could have more than one event read by Emacs?  If so, that's not what
I see: I see exactly one event for each change of the last-access
time.

Or should I try something else?  I can easily post the code and the
procedure I used, if that would help clarify the issue.

Thanks.