exec: report a runtime failure to the parent

[kolyshkin]

When the runtime fails to start the exec'd process, it does not write the pid file. Conmon merely warned about that and exited, and since the failing conmon is the daemonized child, its exit status went nowhere: the caller saw the parent exit 0 and got nothing on the sync pipe, so an exec that never ran looked exactly like a successful one.

A non-zero runtime exit status can not be used to tell those apart for exec -- it is also how the exit status of the exec'd command is reported, which is why the existing check skips exec. A missing pid file, on the other hand, means the process was never started.

So report it: write the failure to the sync pipe, together with whatever the runtime printed to its stderr, and exit with an error rather than a warning.

With this, a failing exec produces

{"data": -1, "message": "... exec failed: unable to start container process: exec: \"/no/such/binary\": stat /no/such/binary: no such file or directory\n"}

instead of silence. A test for that is added.

[TomSweeneyRedHat]

I'm not 100% sure on this one, maybe this instead?

Suggested change

	nexitf("Failed to read pidfile: %s", err->message);
	nexitf("Failed to read pidfile: %s", error_msg);

[kolyshkin]

Nah, those are two different errors:

• err->message is the original error from Glib (something like "Failed to open file “/tmp/conmon-test-XXX/pidfile.2”: No such file or directory"), i.e. the conmon error.
• error_msg is redefined inside if to be the actual runtime's stderr (something like "exec failed: unable to start container process: ...")
  and we want to see both the way we reveal those.

[TomSweeneyRedHat]

One question, otherwise LGTM

[jnovy]

LGTM. Clean, well-motivated fix for a real silent-failure bug.

The problem: when the runtime fails to start an exec'd process (e.g., binary does not exist), it never writes the pid file. The old code just warned and called exit(1), but since this runs in the daemonized child, the exit status is lost - the parent saw a successful exit and got nothing on the sync pipe, making a failed exec indistinguishable from a successful one.

The fix: detect the missing pidfile, read whatever the runtime wrote to stderr, report it via the sync pipe as {"data": -1, "message": "..."}, and exit with an error. This is the right detection signal for exec because (as the comment explains) non-zero runtime exit status is ambiguous for exec - it also conveys the exec'd command's own exit code.

Code review notes:

• Nonblocking read: g_unix_set_fd_nonblocking(mainfd_stderr, ...) before the read is correct defensive coding. Notably, the existing stderr-reading block above (line 366 on main) does not set nonblocking, which could theoretically block - this new code is actually better in that regard.

• sync_pipe_fd >= 0: correct, since fd 0 is a valid file descriptor. This is consistent with line 407 (opt_api_version >= 1 path). The pre-existing checks at lines 374/385 use > 0, which is a minor inconsistency in the existing code (not introduced by this PR).

• Error message fallback: error_msg defaults to err->message (the pidfile-read error) and is overridden only if runtime stderr has content - good fallback behavior so the sync pipe always carries something useful.

• No resource issues: buf is stack-allocated (BUF_SIZE = 8192), null-terminated after read(), and nexitf() terminates the process so there are no dangling state concerns.

• write_or_close_sync_fd safety: the function internally checks *fd == -1 and returns early, so the >= 0 guard plus the internal check makes this double-safe.

• Test: well-structured, follows the existing patterns in 08-exec.bats. The /no/such/binary approach to trigger a runtime failure is clean, and asserting both "data": -1 and "exec failed" validates both the structure and the content of the error report.

[jnovy]

LGTM - clean, well-motivated fix for a real silent-failure bug. The error propagation via sync pipe is correct, nonblocking read is properly set up, and the test covers the failure path well.