fix(terminal): interrupt/got_int hangs terminal

[ibhagwan]

terminal_execute should always clear got_int or safe_vgetc will keep returning Ctrl_C in an endless loop.

Minimal reproduction of the issue:

will cause neovim to hang with 100% CPU prior to this patch

:autocmd TermEnter * call timer_start(500, {-> interrupt()})
:new | term
:startinsert

Closes #20726

[justinmk]

please includes the relevant parts of your analysis, in the commit message. and also the comment from https://github.com/neovim/neovim/issues/20726#issuecomment-2290574907 is helpful:

The are some other places that can set got_int. For example, the hang can happen when calling interrupt() in Terminal mode. So got_int needs to be cleared more often

[justinmk]

Minimal reproduction of the issue:

can you add a test? test/functional/terminal/buffer_spec.lua looks like a good place

[ibhagwan]

Minimal reproduction of the issue:

can you add a test? test/functional/terminal/buffer_spec.lua looks like a good place

Sure, I’ll have to do some exploration of the testing codebase to find out how (if?) to test that the process isn’t frozen, if you can point me towards an existing similar test (if a similar test exists) would be very helpful and save me some time :)

[zeertzjq]

Maybe you can just use feed() and then screen:expect() to check if the process is responding to input.

[ibhagwan]

Maybe you can just use feed() and then screen:expect() to check if the process is responding to input.

I wasn't able to run the tests locally yet (which I needed to examine screen:expect) so I thought of a different way to test this PR, stolen from the ModeChanged tests, using the autocmd that sends a delayed interrupt if the process is frozen we won't be able to return from terminal (insert) mode to normal mode.

https://github.com/neovim/neovim/pull/30056/commits/7e41baf6ec57ee2daff7371b8bddb15ad551e0cf

[justinmk]

confirmed that the test case fails correctly, though it hangs (so screen:expect() would be ideal, since it handles hangs better)

if there are no further comments, ping me in a few days to merge this

[ibhagwan]

confirmed that the test case fails correctly, though it hangs (so screen:expect() would be ideal, since it handles hangs better)

Them @justinmk!

if there are no further comments, ping me in a few days to merge this

Will do, have a great weekend.

[ibhagwan]

Tysm @zeertzjq for simplifying the CI!

@justinmk, would be a great start of the week when this gets merged and I can close the only lingering issue in fzf-lua :)

[neovim-backports[bot]]

Successfully created backport PR for release-0.10:

• #30094

[ibhagwan]

Ty for all the help @zeertzjq @justinmk walking me through this 🙏