search

oxidecomputer / helios

Helios: Or, a Vision in a Dream. A Fragment.
Mozilla Public License 2.0
371 stars 10 forks source link

Ensure that rust's std sets MSG_NOSIGNAL for send(2) on illumos #148

Closed sunshowers closed 4 months ago

sunshowers commented 7 months ago

(This is a general rust stdlib + illumos issue, filing here for tracking.)

tl;dr: rustc doesn't pass in MSG_NOSIGNAL on illumos -- we should fix that.

On Unix systems, by default, writing to a pipe that is broken causes two things to happen:

  1. A SIGPIPE signal is raised.
  2. An EPIPE error is returned.

The default disposition of SIGPIPE is to terminate the program. This is why things like ls | head terminate cleanly. But Rust sets the SIGPIPE disposition to SIG_IGN by default. There's a long history behind this: see https://github.com/rust-lang/rust/issues/62569 for more.

The upshot of this is that Rust binaries fail with errors like:

BRM42220014 # /opt/oxide/oxlog/oxlog logs --archived oxz_switch | head -n1
/pool/ext/0c4ef358-5533-43db-ad38-a8eff716e53a/crypt/debug/oxz_switch/oxide-dendrite:default.log.1693451383
thread 'main' panicked at library/std/src/io/stdio.rs:1021:9:
failed printing to stdout: Broken pipe (os error 32)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

What we can do, from the perspective of shipping binaries written in Rust, is to simply set SIGPIPE to SIG_DFL for them. But there is a wrinkle: the same logic about broken pipes applies not just to writes to local pipes, but also network connections. While sending data, if there's a connection reset for some reason, SIGPIPE will be raised as well.

The good news is that many modern Unix platforms recognize this fundamental mismatch between what CLI tools want and what network services should do. They address it in one of two ways:

  1. Support a flag to send(2) called MSG_NOSIGNAL (Linux, several BSDs, and illumos).
  2. At socket creation time, set a flag called SO_NOSIGPIPE (Apple platforms, example use).

Focusing on 1 since that's what's relevant to illumos, I spent some time looking at the rustc source code, and saw that rustc does pass in MSG_NOSIGNAL where supported. However, the conditional doesn't include illumos, and so this flag isn't passed in on illumos even though it is supported.

Overall, this means that for a tool like oxlog, setting SIGPIPE to SIG_DFL is fine to do because it doesn't make any network requests (https://github.com/oxidecomputer/omicron/pull/5358 does so). But other CLI tools like wicket, omdb and oxdb do make network requests -- and just exiting if a connection reset happens wouldn't be great.

It would be good to get this fixed in Rust upstream. There are likely tests for this behavior too, which we should ensure are enabled on illumos.

sunshowers commented 4 months ago

Addressed by https://github.com/rust-lang/rust/pull/128259.

sunshowers commented 4 months ago

https://github.com/rust-lang/rust/pull/128259 has landed -- expected to be part of Rust 1.82.