Prevent ctags process stacking on stucked .tags.kaklock folder

[stacyharper]

I'm not sure to fully understand what is causing this, but it happened to me multiple time. A new process get stuck on every buffer save, and I discover at the end of the day that a hundred of them are stucked.

At least let's try to remove the lock folder when starting Kakoune.

[krobelus]

probably it gets stuck in

while ! mkdir .tags.kaklock 2>/dev/null; do sleep 1; done

how about mkdir -p .tags.kaklock || exit ? Maybe also check git blame info for this line

[stacyharper]

The goal for this line is of course to prevent race condition. But in some situations, the folder remains after Kakoune exit. Maybe the folder does not get cleaned up in some cases. I'm not sure. At least with this patch, the folder get cleaned up with a future instance of Kakoune.

[mawww]

Not sure if we should remove that lock file on startup, some people use multiple Kakoune sessions in the same project and this change would break that use case.

Investigating what cases lead to the file still existing would be good, as we currently remove it with trap 'rmdir ...' EXIT which should be pretty robust.

[stacyharper]

the process would never exit, because it wait for the lock. Does the shell process is killed by Kakoune when exiting? Maybe we should add some signals to this trap?

[mawww]

The new process yeah, what I am trying to understand is why the old process exited without removing the lock, or maybe it is actually still running ?

[stacyharper]

because I quited Kakoune while ctags was running? meaning the shell get killed just after ctags, without entering the trap

[mawww]

Should'nt the trap run in this case ? Doing some quick testing here, EXIT traps seem to run on SIGTERM and SIGINT, it does not on SIGKILL but I would be surprised for this shell script to get SIGKILL'd.

[stacyharper]

I got this simple config. What happen when we ":wq" in a large code base folder?

hook global WinSetOption filetype=[^*].+[^*] %{
	hook buffer BufWritePost .* %{ ctags-update-tags }
}

The shell being killed without trapping seems the only way for this issues to occurs to me

[mawww]

Testing here this is what I get:

$ cat test.kak 
hook global BufWritePost .* %{
    nop %sh{ ( trap 'echo > trapped' EXIT; sleep 2s; echo > exited ) >/dev/null 2>&1 </dev/null & }
}
$ rm exited trapped; time kak -e 'source test.kak; e test.kak; wq'; sleep 2; ls exited trapped

real	0m0.087s
user	0m0.074s
sys	0m0.018s
exited  trapped

So, as far as wq is concerned, it should not kill the ctags process but trap should eventually run and remove the lock. Is it possible that ctags generation takes a while on your machine, and what you see is the previous ctags still running ? If that is the case it seems the lock is working as intended, preventing multiple ctags process to update the same file at the same time.

[stacyharper]

I reproduce your outputs here, and I also don't see how this can happen. In this "wq" situation, the script is updating ctags, meaning it only generates ctags for one buffer. So no, this should always run fast. The only way I can think about is, if I run "ctags-generate" and then I poweroff the machine immediately. Even if the poweroffing would take some time, a signal might already have killed the shell process, preventing the EXIT trap to run.

I replaced this hook by a debug warning, and will try to check the debug buffer often.

[stacyharper]

I think I got something. The fact that kakoune does not kill its shell instances, and that we don't trap those signals in them, might still cause issues. Kakoune might be killed using its process group, meaning all its sub-process will also receive the kill signal. In this situation, this would cause the bug because the script would handle the signal just after the ctags completion, and will never reach EXIT. I also think this concretely happen when the terminal emulator is being killed. If I run "ctags-generate", and then I kill my Foot instance, the lock folder is never removed.

edit: I confirm this happen if kakoune is being killed by process group. Reproduction: run two terminals, one prepare a kakoune instance in a big codebase folder, with ":ctags-generate" prepared. In another terminal, run sleep 5 && echo now && sleep 1 && pgrep kak | xargs -I{} kill -- -{}. On "now", press <ret> on Kakoune. The folder remains indefinitely.

[mawww]

This is weird, kill should send SIGTERM by default, so the trap would still trigger. If you send SIGKILL then yes we would leak the lock, but this seems like a very unlikely case, or at least, a case where the bug lies in whatever sends a SIGKILL instead of SIGTERM.

Testing here, with the previous script and making the delay longer, I see the sleep process staying alive after closing the foot window that has Kakoune running directly. How do you kill foot ?

[stacyharper]

There is not trap for TERM, so the script is just stopped early, and the EXIT trap is never reached. This is something easily reproductible with some script as:

#!/bin/sh

trap "notify-send foo" EXIT

sleep 5

Then kill this script while sleeping. It does not run the notify-send.

So imho the scripts that use some lock mechanisms should always handle this signal to prevent those unrecoverable situations. Either because Kakoune would signal it when it stops, because of a process group signal propagation, or just because all process have to stop (poweroff). I'm not sure if we should propagate the signal to the child ctags process. Probably this is not necessary.

edit:

How do you kill foot ?

I use my Sway "kill" keybind to kill the window.

[mawww]

Hum, which shell are you using ? Doing some testing here it seems the EXIT trap runs in bash but not in dash when closing the terminal directly from sway.

[mawww]

https://pubs.opengroup.org/onlinepubs/9699919799/ is not very clear on when EXIT should be triggered, both dash and busybox sh do not seem to trigger when foot is closed directly, it would be good to investigate a bit more why they do not.

[krobelus]

yeah dash needs something like "trap ... EXIT HUP INT QUIT"

[Screwtapello]

https://pubs.opengroup.org/onlinepubs/9699919799/ is not very clear on when EXIT should be triggered...

That links to the 2018 version of POSIX. The 2024 edition says:

The EXIT condition shall occur when the shell terminates normally (exits), and may occur when the shell terminates abnormally as a result of delivery of a signal (other than SIGKILL) whose trap action is the default.

...which I guess just means that both the bash and dash behaviours are allowed.

[mawww]

yeah dash needs something like "trap ... EXIT HUP INT QUIT"

That does not seem to work unfortunately, trap "notify-send exited" EXIT HUP INT QUIT in dash or busybox sh then closing foot does not trap... And interestingly triggers twice in bash...

[stacyharper]

I'm using Bash in Foot, and busybox shell as sh. IIRC it can run twice because both the signal handler, then the exiting are triggering. I once avoided this by untrapping in a finish trap handler. I was not aware of those behavior difference, thanks for pointing that out.

edit:

So what does this means for us? What is the most compatible approach? We should probably not trap EXIT, but run the cleanup linearly after the ctags cmd. And trap signals to finish early?