micro does not die when killing crontab and using micro as editor

[EnderCrypt]

launching crontab with micro as editor, and then killing crontab (the entire terminal) causes micro to stay alive (other editors like vim does not exhibit this behaviour) and micro goes from eating ~0% cpu to eating ~40% cpu forever untill killed
sending a SIGTERM to the process does NOT kill it, but does cause it to drop its cpu usage from ~40% to ~13%
sending a SIGKILL does kill it as expected

how to reproduce

1. open a GNU/Linux terminal of choice (i use konsole)
2. execute EDITOR=`which micro` crontab -e
3. kill the terminal by clicking the [x] (i used xkill instead)
4. open a process monitorer (i use KSysGuard) and sort processes by cpu usage

Specifications

Version: 2.0.8
Commit hash: cfcb2e4
Compiled on February 21, 2021

OS: 5.9.16-1-MANJARO
Terminal: konsole (with zsh)

[dmaluka]

AFAICS 0851499 did not fully fix this issue. Micro process indeed now exits if we kill crontab by closing the terminal window, but:

• It doesn't exit if we kill crontab via killall crontab.
• It doesn't exit by closing the terminal window if it is running a shell job e.g. via RunInteractiveShell().

[niten94]

It doesn't exit if we kill crontab via killall crontab.

I tried searching a bit but I think errno is set as EIO when reading terminal input in background processes in POSIX. The error that would be set in errno in C is returned when calling Screen.PollEvent so I think it can be checked if event.(*tcell.EventError).Err() is *os.SyscallError. It is only printed in DoEvent in cmd/micro/micro.go so micro does not exit when it is in the background:
https://github.com/zyedidia/micro/blob/0fa4a3a8db09f1690dc171fbe8b57dd5f052b35d/cmd/micro/micro.go#L453-L470

It doesn't exit by closing the terminal window if it is running a shell job e.g. via RunInteractiveShell().

I was testing a bit but micro seems to hang when terminal is closed and Screen.Show is called in DoEvent. SIGHUP is received when the terminal is closed so I think checking if signals are received before the screen is displayed can be done. The signal can be ignored in programs that are run so I think functions in internal/shell like RunInteractiveShell would have to return when it is received.

[dmaluka]

This is what nano does in this case:

		/* When we've failed to get a keycode millions of times in a row,
		 * assume our input source is gone and die gracefully.  We could
		 * check if errno is set to EIO ("Input/output error") and die in
		 * that case, but it's not always set properly.  Argh. */
		if (input == ERR && ++errcount == 12345678)
			die(_("Too many errors from stdin\n"));

This is how it looks like:

Too many errors from stdin

Buffer written to /tmp/crontab.x0ZKs2/crontab.save

So among other things, nano indeed dies gracefully (unlike micro), saving the modified buffer to a backup file.

Vim also behaves similarly:

Vim: Error reading input, exiting...
Vim: Finished.

Looks like this is where vim does that:

	if (wtime == -1 && ++count == 1000)
	    read_error_exit();

[niten94]

I was looking at the code of nano and vim a bit but I still think exiting instantly when EIO is returned can be done. wgetch is used when reading input in nano but it is not written in the ncurses manual page of wgetch that errno may be EIO. I think it is not checked in vim because there are platforms where read is not called.

Errors that are not specifically checked can be returned so I think exiting when errors are returned continuously at least 1000 times can be done. I will try fixing the bugs.

Edit: I realized there may be some things wrong with the fix I wrote in this comment so I am thinking about it again.

[niten94]

I tried checking why micro does not exit when reading in the background but I have taken a while when checking again so I do not think I can fix the bug. I do not know much about concurrency but concurrent code may have to be modified when fixing the bugs in a different way.

• It doesn't exit if we kill crontab via killall crontab.

There are no tcell events received after an event error is returned so it cannot be checked if there are errors received continously. I was thinking that the error can be checked if it is EIO, but there are platforms where EIO is not returned so the bug is not properly fixed if only checking that.

Exiting may have to be done when an event error is received but nil and an error is received sometimes when the screen is finalized. I have not checked what hapens when using upstream tcell but the code and behavior of PollEvent and inputLoop seems to be changed. I think clearing events that are in screen.Events in screen.TempFini can be done as a workaround so that errors are not received when the screen is finalized.

• It doesn't exit by closing the terminal window if it is running a shell job e.g. via RunInteractiveShell().

I was not able to check why micro hangs when terminal is closed and Screen.Show is called in DoEvent so I was only able to think about checking received signals before it is called. There may be nothing else that can be done, so I will make a pull request where signals are checked before calling Screen.Show for now.

Edit: I thought that micro was hanging because the screen was being locked in tcell but I tried testing a bit again and realized there may be something wrong done in micro, so I will not create the pull request.