[Pkg-emacsen-addons] Bug#907715: lua-mode: FTBFS in buster/sid ("No buffer named *lua5.3*")

[Sergio Durigan Junior]

Control: tags -1 + patch

On Friday, August 31 2018, Santiago Vila wrote:

> Dear maintainer:
>
> I tried to build this package in buster but it failed:
[...]

I finally had some time to look at this tonight, and I think I found the
problem.  Bear with me.

I wasn't able to reproduce the bug here on my machine, so I asked
Santiago to provide access to the machine where he was seeing the issue
manifest.  He kindly did that (thanks), and then I was able to reproduce
it.

The problem happens especifically on the "does not ask for file on
"stdin:NN" errors" test.  You can notice that the two tests above it,
which are part of the "Fontification..." suite, don't run.  After some
investigation, I noticed that the problem was that 'lua-process-buffer'
points to a killed buffer when the test is invoked, and *remains*
pointing to that buffer throughout the test.  However, when I ran the
test on my machine, I noticed that, even though 'lua-process-buffer'
pointed to the killed buffer when the test started, it was reassigned
during the test and started pointing to an actual valid buffer.  Hm...

I did some more investigation, and noticed that the tests that run
before the problematic one mess with starting and killing the lua
process, and making sure that everything works as expected.  When these
tests end, the lua process is actually killed.  So that prompted me to
look at *why* (lua-send-buffer) wasn't creating a new process, as it
should.

I thens tarted looking at lua-send-region, which calls lua-send-string,
which calls lua-get-create-process...  Oh!  Let's see what it does:

  (defun lua-get-create-process ()
    "Return active Lua process creating one if necessary."
    (or (and (comint-check-proc lua-process-buffer)
             lua-process)
        (lua-start-process))
    lua-process)

This function actually calls comint-check-proc, which is part of
comint.el:

  (defun comint-check-proc (buffer)
    "Return non-nil if there is a living process associated w/buffer BUFFER.
  Living means the status is `open', `run', or `stop'.
  BUFFER can be either a buffer or the name of one."
    (let ((proc (get-buffer-process buffer)))
      (and proc (memq (process-status proc) '(open run stop)))))

Debugging this function allowed me to determine the difference between
what was happening on my machine and what was happening on Santiago's
machine.  The problem was that, on my machine, (process-status proc) was
returning 'signal (which is the correct value, meaning that the lua
process had received a signal and exited), whereas on Santiago's machine
it was returning 'run (wrong!).  But, why was that?

I looked at Emacs's source code, where the "process-status" function is
defined in C, and started thinking that this may be a race condition.
When the test kills the lua process, Emacs sends a signal, and the lua
process has to exit gracefully.  This may take some time, depending on
what the process does.  Meanwhile, the testcase keeps running and hits
the problematic test, which checks if the process is still running, and
Emacs tells that it is, which prevents lua-mode from actually creating
another process.

The solution that worked for me, at least on Santiago's computer, is to
sleep 1 second before actually running the test.  This was able to give
the lua process time to properly finish, so Emacs was finally aware of
it.

One could argue that 1 second may be too little.  That may be right.
Another solution would be to explicitly call (lua-start-process) when
the test starts, which doesn't seem to have any side effects.  But I
chose the simpler path, and didn't want to modify the test more than I
needed.

Here's the debdiff, with everything needed to upload it.  Feel free to
give it a try, of course, and let me know if it works for your other
machines.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

diff -Nru lua-mode-20151025/debian/changelog lua-mode-20151025/debian/changelog
--- lua-mode-20151025/debian/changelog	2017-12-14 05:23:34.000000000 -0500
+++ lua-mode-20151025/debian/changelog	2019-05-03 19:02:02.000000000 -0400
@@ -1,3 +1,12 @@
+lua-mode (20151025-3.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Sleep before running 'does not ask for file on "stdin:NN" errors" test
+    in order to avoid race condition on (process-status) when running on
+    slow machines (Closes: #907715).
+
+ -- Sergio Durigan Junior <sergiodj at debian.org>  Fri, 03 May 2019 19:02:02 -0400
+
 lua-mode (20151025-3) unstable; urgency=medium
 
   * Change tests to work with buttercup 1.9, thanks to buttercup author
diff -Nru lua-mode-20151025/debian/patches/0003-process-status-race-sleep-bug-907715.patch lua-mode-20151025/debian/patches/0003-process-status-race-sleep-bug-907715.patch
--- lua-mode-20151025/debian/patches/0003-process-status-race-sleep-bug-907715.patch	1969-12-31 19:00:00.000000000 -0500
+++ lua-mode-20151025/debian/patches/0003-process-status-race-sleep-bug-907715.patch	2019-05-03 19:00:40.000000000 -0400
@@ -0,0 +1,35 @@
+Description: Sleep for 1 second to avoid race condition on (process-status)
+ On (very?) slow computers, we might have a
+ race-condition here when trying to obtain the status
+ of the (now dead) Lua process, and Emacs might say
+ that it is still running, when it's actually not.
+ That's why we sleep for 1 second.
+ .
+ For more info, please see
+ <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=907715>.
+Author: Sergio Durigan Junior <sergiodj at debian.org>
+Bug-Debian: https://bugs.debian.org/907715
+
+---
+
+Bug-Debian: https://bugs.debian.org/907715
+Last-Update: 2019-05-03
+
+--- lua-mode-20151025.orig/test/test-inferior-process.el
++++ lua-mode-20151025/test/test-inferior-process.el
+@@ -118,6 +118,15 @@ function () end
+       (unwind-protect
+           (progn
+             (save-current-buffer
++	      ;; On (very?) slow computers, we might have a
++	      ;; race-condition here when trying to obtain the status
++	      ;; of the (now dead) Lua process, and Emacs might say
++	      ;; that it is still running, when it's actually not.
++	      ;; That's why we sleep for 1 second.
++	      ;;
++	      ;; For more info, please see
++	      ;; <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=907715>.
++	      (sleep-for 1)
+               (setq buf (find-file fname))
+               (insert "function () end")
+               ;; Make sure the buffer can be killed cleanly
diff -Nru lua-mode-20151025/debian/patches/series lua-mode-20151025/debian/patches/series
--- lua-mode-20151025/debian/patches/series	2017-12-14 05:23:34.000000000 -0500
+++ lua-mode-20151025/debian/patches/series	2019-05-03 19:02:02.000000000 -0400
@@ -1,2 +1,3 @@
 0001-Change-lua-default-application-for-inferior-process-.patch
 0002-Adjust-tests-to-deal-with-incompatible-changes-made-.patch
+0003-process-status-race-sleep-bug-907715.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-emacsen-addons/attachments/20190503/09ff8496/attachment.sig>