Currently, when quitting MacVim using Cmd-Q, if an autocmd queries serverlist() during shutdown (e.g. VimLeavePre), there's a potential that Vim will crash and then stuck in a spinloop and never gets killed by the parent process.
The reason is somehwat complicated. MMAppController tells Vim to quit but has a hard-coded timer before terminating the connection. If Vim takes too long to respond somehow, it will see a "connectionDidDie" message where it will be forced to quit. The serverlist() IPC API call isn't properly guarding against an invalid connection and if an autocmd triggers that call during this time, it will throw an exception and crash.
Usually if Vim crashes, it should terminate cleanly, but couple things cause this to not work properly:
Vim's signal handler deathtrap tries to exit cleanly when a signal is detected, and it tries to call all deferred functions (added by :defer in Vimscript). The list of functions are allocated on the stack rather than the heap.
The ObjC exception is thrown inside a CFRunLoop (which is what called connectionDidDie) and CFRunLoop silently handles the exception before re-throwing it which triggers the actual abort signal to be caught by Vim's signal handler, but at this time, the deferred functions data structure is messed up as the stack is already gone since the first exception unwound it. This leads to a bogus memory state and lead to an infinite loop in invoke_all_defer.
MacVim also doesn't have a solid mechanism to shut down zombie processes right now (it depends on Vim cleaning up after itself), so after MacVim quits, the now-orphaned Vim process stuck consuming 100% CPU.
The fix is to simply guard against this and make sure we clean up the connection properly when we detected it died, and to be more defensive and make sure the serverlist call properly guard against invalid states and exceptions.
Not tackling the other issues for now. There are some unfortunate interactions here with an unwound exception causing invoke_all_defer() to not work, but we just need to make sure to guard potential places with try/catch blocks, as invoke_all_defer() is still useful. Also, proper zombie process killing will be done at a later time, as we will soon tackle removing Distributed Objects/NSConnection and revamp the entire IPC stack anyway.
Currently, when quitting MacVim using Cmd-Q, if an autocmd queries serverlist() during shutdown (e.g. VimLeavePre), there's a potential that Vim will crash and then stuck in a spinloop and never gets killed by the parent process.
The reason is somehwat complicated. MMAppController tells Vim to quit but has a hard-coded timer before terminating the connection. If Vim takes too long to respond somehow, it will see a "connectionDidDie" message where it will be forced to quit. The serverlist() IPC API call isn't properly guarding against an invalid connection and if an autocmd triggers that call during this time, it will throw an exception and crash.
Usually if Vim crashes, it should terminate cleanly, but couple things cause this to not work properly:
deathtraptries to exit cleanly when a signal is detected, and it tries to call all deferred functions (added by :defer in Vimscript). The list of functions are allocated on the stack rather than the heap.invoke_all_defer.MacVim also doesn't have a solid mechanism to shut down zombie processes right now (it depends on Vim cleaning up after itself), so after MacVim quits, the now-orphaned Vim process stuck consuming 100% CPU.
The fix is to simply guard against this and make sure we clean up the connection properly when we detected it died, and to be more defensive and make sure the serverlist call properly guard against invalid states and exceptions.
Not tackling the other issues for now. There are some unfortunate interactions here with an unwound exception causing invoke_all_defer() to not work, but we just need to make sure to guard potential places with try/catch blocks, as invoke_all_defer() is still useful. Also, proper zombie process killing will be done at a later time, as we will soon tackle removing Distributed Objects/NSConnection and revamp the entire IPC stack anyway.
Fix #1423