GNU bug report logs - #31981
26.1; flymake: removes temporary file before finishing check

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31981 in the body.
You can then email your comments to 31981 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Enrico Scholz <enrico.scholz <at> sigma-chemnitz.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1; flymake: removes temporary file before finishing check
Date: Tue, 26 Jun 2018 19:26:13 +0200

flymake gets confused and terminates itself when checking needs a long
time.  It can be reproduced with a makefile like

--- Makefile ---
check-syntax:
	sleep 2
	${MAKE} ${CHK_SOURCES}
----------------

1. start 'emacs -Q'

2. open 'foo.c' in the directory with the makefile above

3. M-x flymake-mode

4. start typing

   --> "Wait" appears in status line

5. continue typing

   --> "!" appears in status line and log reports


  | Warning [flymake-proc foo.c]: deleted file /tmp/YYY/foo_flymake.c
  | Warning [flymake-proc  *flymake output for foo.c*-919402]: Reference to file e: *** [Makefile is out of scope
  | Warning [flymake-proc  *flymake output for foo.c*-131024]: Reference to file e: *** [Makefile is out of scope
  | Warning [flymake-proc foo.c]: process 2803326 exited with code 2
  | Error [flymake-proc foo.c]: :configuration-error: Command (make -s -C ./ CHK_SOURCES=foo_flymake.c SYNTAX_CHECK_MODE=1 check-syntax) errored, but no diagnostics
  | Warning [flymake foo.c]: Disabling backend flymake-proc-legacy-flymake because :configuration-error: Command (make -s -C ./ CHK_SOURCES=foo_flymake.c SYNTAX_CHECK_MODE=1 check-syntax) errored, but no diagnostics




--------------
GNU Emacs 26.1 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 2.24.32)
 of 2018-05-31 built on koji-builder4.intern.sigma-chemnitz.de
Windowing system distributor 'Fedora Project', version 11.0.11906000
System Description:	Fedora release 28 (Twenty Eight)

...

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk
 --with-gpm=no --with-modules build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong
 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY ACL
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK2 X11 MODULES THREADS LCMS2

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: de_DE.UTF-8
  locale-coding-system: utf-8-unix

Message #8 received at 31981 <at> debbugs.gnu.org (full text, mbox):

From: Joel Rosdahl <joel <at> rosdahl.net>
To: 31981 <at> debbugs.gnu.org
Subject: Fix 'flymake-proc-legacy-flymake' temporary file deletion bug
Date: Tue, 27 Aug 2019 22:35:04 +0200

[Message part 1 (text/plain, inline)]

I can reproduce this bug on latest master.

The problem is that if a check command takes a long time, the
following can happen:

1. The user modifies the buffer, making flymake-proc start a process A
which reads from a temporary file T containing the buffer content.
2. Before process A has finished, the user modifies the buffer again,
making flymake-proc start a new process B reading from the same
temporary file T.
3. A is marked as obsolete.
4. When A's sentinel detects that A has died, it runs A's cleanup
function which deletes T.
5. B may fail since T is gone.

Attached is a patch which solves the issue, at least for me: simply
don't run the cleanup function for an obsolete process. This should
work well for checks that use flymake-proc-simple-cleanup or one of
the other cleanup functions defined in flymake-proc.el.

I'm not sure if the fix is appropriate for the general case with a
user-supplied custom cleanup function, though. But I see no other
quick fix for the issue since the name of the temporary file is kept
in a single buffer-local variable and there is no way for the cleanup
function to know that it's being called for an obsolete process.

-- Joel

[0001-Fix-flymake-proc-temporary-file-deletion-bug.patch (text/x-patch, attachment)]

Message #11 received at 31981 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Joel Rosdahl <joel <at> rosdahl.net>
Cc: 31981 <at> debbugs.gnu.org
Subject: Re: bug#31981: Fix 'flymake-proc-legacy-flymake' temporary file
 deletion bug
Date: Tue, 27 Aug 2019 19:37:56 -0400

Joel Rosdahl <joel <at> rosdahl.net> writes:

> 1. The user modifies the buffer, making flymake-proc start a process A
> which reads from a temporary file T containing the buffer content.
> 2. Before process A has finished, the user modifies the buffer again,
> making flymake-proc start a new process B reading from the same
> temporary file T.

If I'm reading flymake-proc-init-create-temp-buffer-copy correctly, it
actually makes a new temporary file (say T1), but the file name is
stored in a single variable, so Emacs effectively forgets about the
original file T.

> 3. A is marked as obsolete.
> 4. When A's sentinel detects that A has died, it runs A's cleanup
> function which deletes T.

So here it deletes T1, leaving the original T.  And then B will fail,
since it needs T1, not T.

> But I see no other quick fix for the issue since the name of the
> temporary file is kept in a single buffer-local variable and there is
> no way for the cleanup function to know that it's being called for an
> obsolete process.

Would it work to store the filename in the process object instead, with
process-put (and then the sentinel can retreive it with process-get)?

Message #14 received at 31981 <at> debbugs.gnu.org (full text, mbox):

From: Joel Rosdahl <joel <at> rosdahl.net>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31981 <at> debbugs.gnu.org
Subject: Re: bug#31981: Fix 'flymake-proc-legacy-flymake' temporary file
 deletion bug
Date: Wed, 28 Aug 2019 21:18:33 +0200

[Message part 1 (text/plain, inline)]

On Wed, 28 Aug 2019 at 01:38, Noam Postavsky <npostavs <at> gmail.com> wrote:
> If I'm reading flymake-proc-init-create-temp-buffer-copy correctly, it
> actually makes a new temporary file (say T1), but the file name is stored in
> a single variable, so Emacs effectively forgets about the original file T.

The create-temp-f parameter to flymake-proc-init-create-temp-buffer-copy is in
practice flymake-proc-create-temp-inplace, which always creates the same result
for a given file-name and prefix. And file-name and prefix are the same for
both modification A and B (since it's the same file). Or am I missing
something?

> So here it deletes T1, leaving the original T. And then B will fail, since it
> needs T1, not T.

Nope, it's T both times, but it's true that only generating a unique filename
wouldn't fix the issue since, as you said, the filename is stored in a single
variable.

> Would it work to store the filename in the process object instead, with
> process-put (and then the sentinel can retreive it with process-get)?

Yes, flymake-proc-create-temp-inplace could create a unique filename and store
it in the process, but that wouldn't help since the cleanup function doesn't
have access to the process (since the process is stored in a single variable as
well). Passing the process or filename(s) to the cleanup function would work,
but it would break the interface to existing custom cleanup functions which
don't take a parameter, so I wrote off that solution earlier.

...unless we can pass the value(s) using dynamic binding to the cleanup
function. I didn't consider this before since lexical binding would prevent
such a solution, but I just learnt to my surprise that global defvar-ed
variables are still dynamically scoped.

Attached is a new proposed solution as sketched above.

-- Joel

[0001-Fix-flymake-proc-temporary-file-deletion-bug.patch (text/x-patch, attachment)]

Message #17 received at 31981 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Joel Rosdahl <joel <at> rosdahl.net>
Cc: 31981 <at> debbugs.gnu.org
Subject: Re: bug#31981: Fix 'flymake-proc-legacy-flymake' temporary file
 deletion bug
Date: Thu, 29 Aug 2019 12:04:57 -0400

Joel Rosdahl <joel <at> rosdahl.net> writes:

> On Wed, 28 Aug 2019 at 01:38, Noam Postavsky <npostavs <at> gmail.com> wrote:
>> If I'm reading flymake-proc-init-create-temp-buffer-copy correctly, it
>> actually makes a new temporary file (say T1), but the file name is stored in
>> a single variable, so Emacs effectively forgets about the original file T.
>
> The create-temp-f parameter to flymake-proc-init-create-temp-buffer-copy is in
> practice flymake-proc-create-temp-inplace, which always creates the same result
> for a given file-name and prefix. And file-name and prefix are the same for
> both modification A and B (since it's the same file). Or am I missing
> something?

No, it's just me who missed this.  There's a lot of indirection, and I
didn't read that carefully.

> but I just learnt to my surprise that global defvar-ed
> variables are still dynamically scoped.

Well, they would be unusable otherwise, since global vars don't have any
lexical scope as such.

> +                     ;; Make cleanup-f see the temporary file names
> +                     ;; created by its corresponding init function
> +                     ;; (bug#31981).
> +                     (let ((flymake-proc--temp-source-file-name
> +                            (process-get proc 'flymake-proc--temp-source-file-name))
> +                           (flymake-proc--temp-master-file-name
> +                            (process-get proc 'flymake-proc--temp-master-file-name)))
> +                       (funcall cleanup-f)))))

A little awkward, but that's proably the best we can do without changing
the signature of cleanup-f (which I guess would be much more
troublesome).

Message #20 received at 31981 <at> debbugs.gnu.org (full text, mbox):

From: Joel Rosdahl <joel <at> rosdahl.net>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31981 <at> debbugs.gnu.org
Subject: Re: bug#31981: Fix 'flymake-proc-legacy-flymake' temporary file
 deletion bug
Date: Thu, 29 Aug 2019 19:30:31 +0200

On Thu, 29 Aug 2019 at 18:05, Noam Postavsky <npostavs <at> gmail.com> wrote:
> A little awkward, but that's proably the best we can do without changing
> the signature of cleanup-f (which I guess would be much more
> troublesome).

I agree. Alternative solutions welcome, of course.

-- Joel

Message #23 received at 31981 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Joel Rosdahl <joel <at> rosdahl.net>
Cc: 31981 <at> debbugs.gnu.org
Subject: Re: bug#31981: Fix 'flymake-proc-legacy-flymake' temporary file
 deletion bug
Date: Sat, 07 Sep 2019 17:05:12 -0400

tags 31981 fixed
close 31981 27.1
quit

Joel Rosdahl <joel <at> rosdahl.net> writes:

> On Thu, 29 Aug 2019 at 18:05, Noam Postavsky <npostavs <at> gmail.com> wrote:
>> A little awkward, but that's proably the best we can do without changing
>> the signature of cleanup-f (which I guess would be much more
>> troublesome).
>
> I agree. Alternative solutions welcome, of course.
>
> -- Joel

None seems forthcoming, so I've pushed your patch to master (I coalesced
your ChangeLog entries, by the way).

8d588f09e9 2019-09-07T16:51:24-04:00 "Fix flymake-proc temporary file deletion bug"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8d588f09e91e315c715cf824a9819a538a85cd9c

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.