bazelbuild / bazelbuild/remote-apis

Let exit_code be better aligned with C/POSIX

Open
#240 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
445
Forks
141
PR merge metrics
No merged PRs in 30d

Description

I am glad that REv2 already uses `int32` as its data type for `exit_code`. Even though most UNIX systems historically only had 8-bit exit codes, modern versions of POSIX provide interfaces for obtaining the full integer value. Using functions like [`waitid()`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitid.html), you can obtain a [`siginfo_t`](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html) whose `si_status` field contains the full value.

Where we do deviate from C/POSIX is that there are no facilities for indicating that a process did not terminate by calling `_Exit()`, but through a signal. This leads to confusing situations where people wonder why their build actions terminate with exit code 139, while in reality it terminated through `SIGSEGV` (signal 11, giving exit code 128+11=139). Even worse are modern versions of Go's `os/exec` package, which will always return an exit code of -1 in case of signal delivery. This means that users can't even figure out why their actions terminated.

One solution to this would be to change the existing field:
```proto
// The exit code of the command.
int32 exit_code = 4;
```
To something like this:
```proto
oneof termination_reason {
// The command terminated by calling the operating system's equivalent
// of `_Exit()` with the code provided.
int32 exit_code = 4;

// The command terminated by the delivery of a signal.
int32 signal_number = 13;
}
```
Even though virtually all operating systems use 9 for `SIGKILL` and 15 for `SIGTERM`, C/POSIX make no such requirement. The only place where that does happen, is in the definition of [the `kill` utility](https://pubs.opengroup.org/onlinepubs/9699919799/utilities/kill.html), where as part of the X/Open System Interfaces (XSI) they provide a hardcoded list of numerical command line options.

My recommendation would thus be to use this instead:
```proto
oneof termination_reason {
// The command terminated by calling the operating system's equivalent
// of `_Exit()` with the code provided.
int32 exit_code = 4;

// The command terminated by the delivery of a signal. This field contains the name of
// the signal, with the "SIG" prefix removed (e.g., "SEGV" for segmentation violations).
string signal_name = 13;
}
```

Unfortunately it's not safe for us to add a `oneof` to the protocol retroactively, as it would cause `exit_code` to be zero when `signal_name` is set. Maybe we can already add this to REv2 in the form of a separate field, with the remark that `exit_code` is set to a non-zero value if `signal_name` is set...

Contributor guide

Open the contributing guide

Research direction

Review the REv2 protocol definition for exit_code and the existing process-result handling; the issue names no files or tests. Compare the termination behavior with POSIX waitid() and Go's os/exec. Done means agreeing on a backward-compatible representation of signal termination and documenting the resulting field semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, operating-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.