1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 01:48:00 +00:00

pterm.exe: fix handling of Windows exception codes.

Unlike on Unix, a Windows process's exit status is a DWORD, i.e. a
32-bit unsigned integer. And exit statuses seen in practice can range
up into the high half of that space. For example, if a process dies of
an 'illegal instruction' exception, then the exit status retrieved by
GetExitCodeProcess will be 0xC000001D == STATUS_ILLEGAL_INSTRUCTION.

If this happens to the process running inside pterm.exe, then
conpty->exitstatus will be set to a value greater than INT_MAX, which
will cause conpty_exitcode to return -1. Unfortunately, a -1 return
from conpty_exitstatus is treated by front ends as saying that the
backend process hasn't exited yet, and is still running. So pterm will
sit around after its subprocess has already terminated, contrary to
your 'close on exit' setting.

Moreover, when cmd.exe exits, it apparently passes on to its parent
process the exit status of the last subcommand it ran. So if you run a
Windows pterm containing an ordinary interactive console session, and
the last subprogram you happen to run inside that session dies of a
fatal signal, then the same thing will happen after you type 'exit' at
the command prompt.

This has been happening to me intermittently ever since I created
pterm.exe in the first place, and I guessed completely wrong about the
cause (I feared some kind of subtle race condition in pterm's use of
the process API). I've only just managed to reproduce it reliably
enough to debug, and I'm relieved to find it's much simpler than that!

The immediate fix, in any case, is to ensure we don't return -1 from
conpty_exitcode unless the process really is still running. And don't
return INT_MAX either, because that indicates 'unclean exit' in a way
that triggers 'close window only on clean exit' (and even Unix pterm
doesn't consider the primary subprocess dying of a signal to count as
unclean). So we clip all out-of-range Windows exception codes to
INT_MAX-1.

In the longer term I think it would be nice to turn exit codes into a
full 32-bit space, and move the special values completely out of it.
That would permit actually keeping the exact exception code and
passing it on to a caller who needed it. For example, if we were to
write a Windows psusan (which I could imagine being occasionally
useful), this way it would be able to return the unmodified full
Windows exit code via the "exit-status" chanreq. But I don't think we
currently have any clients needing that much detail, so that's a more
intrusive cleanup for a later date.
This commit is contained in:
Simon Tatham 2022-03-08 18:05:48 +00:00
parent 0613ec9986
commit ee987ce4cd

View File

@ -356,12 +356,26 @@ static int conpty_exitcode(Backend *be)
{
ConPTY *conpty = container_of(be, ConPTY, backend);
if (conpty->exited &&
0 <= conpty->exitstatus &&
conpty->exitstatus <= INT_MAX)
return conpty->exitstatus;
else
if (conpty->exited) {
/*
* PuTTY's representation of exit statuses expects them to be
* non-negative 'int' values. But Windows exit statuses can
* include all those exception codes like 0xC000001D which
* convert to negative 32-bit ints.
*
* I don't think there's a great deal of use for returning
* those in full detail, right now. (Though if we ever
* connected this system up to a Windows version of psusan or
* Uppity, perhaps there might be?)
*
* So we clip them at INT_MAX-1, since INT_MAX is reserved for
* 'exit so unclean as to inhibit Close On Clean Exit'.
*/
return (0 <= conpty->exitstatus && conpty->exitstatus < INT_MAX) ?
conpty->exitstatus : INT_MAX-1;
} else {
return -1;
}
}
static int conpty_cfg_info(Backend *be)