activation: do not discard unknown descriptors #52
Conversation
This tweaks FD-receiving logic to account for file descriptors of an unknown type (possibly invalid, too). Previously this library would just halt and catch fire upon encountering a single FD for which type-detection failed It now logs a warning message but keeps the FD internally in a dedicated "unknown" variant, allowing consumers to recover it and further try to process in some way.
src/activation.rs
Outdated
| @@ -244,6 +254,13 @@ impl IntoRawFd for FileDescriptor { | |||
| SocketFd::Inet(fd) => fd, | |||
| SocketFd::Unix(fd) => fd, | |||
| SocketFd::Mq(fd) => fd, | |||
| SocketFd::Unknown(fd) => { | |||
| log::warn!( | |||
| "extracting possibly invalid or unknown file descriptor {}", | |||
There was a problem hiding this comment.
In addition to my disfavour of logging from the library, the consumer app should already understand the risks of getting the raw fd, because it erases the info about the FD. The library couldn't know how the FD would be used, so it should not be the library's concern, e. g. a Unix socket FD might be mistakenly used for a TCP socket. The library does not worry about that, so why does it for a Unknown?
There was a problem hiding this comment.
Because in the other cases the FD is known to be valid by the library. But you have a fair point that this is a generally risky/unsafe operation.
I'm going to drop this warning for now, and try to come up later on with a way to convey this detail at the type level with a mix of TryInto and unsafe, in place of this IntoRawFd shotgun.
|
@ip1981 added a fixup commit on top, PTAL. |
| pub fn receive_descriptors_with_names( | ||
| unset_env: bool, | ||
| ) -> Result<Vec<(FileDescriptor, String)>, SdError> { | ||
| let pid = env::var("LISTEN_PID"); | ||
| let fds = env::var("LISTEN_FDS"); | ||
| let names = env::var("LISTEN_FDNAMES"); | ||
| log::trace!( | ||
| "LISTEN_PID = {:?}; LISTEN_FDS = {:?}; LISTEN_FDNAMES = {:?}", |
There was a problem hiding this comment.
I wonder if one line per variable would be better :)
There was a problem hiding this comment.
Possibly, but I don't expect much output from this. Longer term idea is to use a single log entry with structured fields when that is stabilized.
Yes, there a few lines of code duplication, but I've had the de-duplicated version too in the past and it didn't help readability, so I'll stick to this. (I know this is a bit of subjective taste, sorry :)
This tweaks FD-receiving logic to account for file descriptors of an
unknown type (possibly invalid, too).
Previously this library would just halt and catch fire upon encountering
a single FD for which type-detection failed It now logs a warning message
but keeps the FD internally in a dedicated "unknown" variant, allowing
consumers to recover it and further try to process in some way.
Closes: #46