I've been chasing down a bug in vim, where killing vim (usually via SIGHUP
or SIGTERM) causes it to get a SIGSEGV and dump core. I'm doing my work on
Solaris, but I can get it to do the same thing on Ubuntu 14.04, so it's not
strictly a Solaris thing.
Most reproducibly (for me, at least), it seems to be due to the BufWinLeave
autocommand that fugitive sets up. That would make sense to me -- running
autocommands in a signal handler seems to me like it would be inherently
difficult to get right, and certainly not generically possible.
It's running the autocommands from the the HUP/TERM handler, doing
something unsafe, and getting a SIGSEGV. If my understanding of the code
is correct, the design is to handle the SEGV, try cleaning up again (but
skipping the autocommands this time), and if anything goes wrong, rely on
getting another SEGV, and really quitting at that point. Is my
understanding of that correct? But (at least on Solaris), that second SEGV
isn't caught; it just gets delivered, and we get the coredump.
One complication seems to be that if I compile with optimization (-xO4 on
the Studio compilers), then it doesn't seem to catch even the first SEGV,
but just dumps core somewhere in the middle of handling the HUP/TERM. If I
turn off optimization and turn on debugging, then it works as intended,
modulo that the secondary SEGV isn't caught. I have some more work to do
to figure out why the compilation flags make a difference here.
We could try to make this secondary SEGV handling async-signal-safe, but
that seems to me like it's just trying to polish an inherently broken
design.
The thought I have was to do the classic thing where the signal handler
sets a global flag and returns immediately. For the majority of the cases
I've seen, the loop in RealWaitForChar is where the signal hits anyway, so
checking for that flag there would be easy, and it could call
preserve_exit() and exit quickly and sanely, even if it can't do a lot of
cleanup. The simple (and incomplete) attached patch works for my testcase.
Other places would be harder to handle, but maybe they don't happen enough
to be worth it.
Thoughts?
Thanks,
Danek
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
diff --git a/src/os_unix.c b/src/os_unix.c
index b4808b5..0a15073 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -1119,6 +1119,8 @@ deathtrap SIGDEFARG(sigarg)
sprintf((char *)IObuff, "Vim: Caught deadly signal\n");
#endif
+ SIGRETURN;
+
/* Preserve files and exit. This sets the really_exiting flag to prevent
* calling free(). */
preserve_exit();
@@ -5775,6 +5777,8 @@ select_eintr:
* SIGWINCH. */
if (do_resize)
handle_resize();
+ if (get_vim_var_nr(VV_DYING) >= 1)
+ preserve_exit();
/* Interrupted by a signal, need to try again. We ignore msec
* here, because we do want to check even after a timeout if
