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 <sergi...@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 <sergi...@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
signature.asc
Description: PGP signature