SetTimeFlags::TimerCancelOnSet is missing a proper interface

[main--]

The current implementation of TimerFd::read() assumes that we can always obtain the number of timer expirations somehow. The whole point of enabling TimerCancelOnSet however is to introduce a case where we don't get this number.

Right now this effectively means that the only way to use this flag is to wrap the read() call in catch_panic and treat the panic case as cancellation. Clearly, this is awful.

Idea: add read_cancelable() -> Option<u64>

[petreeftime]

Would you consider a solution that makes read() not panic at all and return a Result instead? The interface right now panics if it reads a 0, and if the result is anything expect the two error cases defined there and Ok(8), which doesn't cover all the possible cases. Ok(0) is possible according to the spec, but also there might be some other errors returned. I think that the user of the library might not want a panic.

[main--]

I'm reluctant to returning a generic io Result because IMO that would be a leaky abstraction: suddenly the user of this crate has to check the documentation of the timerfd API to figure out potential error return values and how they could be handled. Obviously it's more flexible, but this crate already does so little (especially now that it just wraps calls to rustix's timerfd functions) that abstracting over read return values is one of the few areas where it can provide real value.

I don't recall the bit about CLOCK_REALTIME and discontinous negative clock changes, but aside from that even in today's manpage I don't see the possibility of read() returning anything but 8 or error.

Looking at the tradeoffs here, I still belive that there should be two read methods. The panicking that exists today is very useful for most usecases. If you need cancellation, or if you're using the realtime clock (which I've never seen a usecase for, but if you do it, you're subject to the zero return), then we can provide an alternate read function for this. Since Ok(0) should probably be translated to Result::Err here, I think that a new error enum would be best. E.g. something roughly like:

#[derive(Snafu)]
pub enum TimerfdReadError {
    #[snafu(display("Timer was cancelled"))]
    TimerCanceled,
    /// Only happens for timers using `ClockId::Realtime`.
    #[snafu(display("System time went backwards"))]
    RealtimeWentBackwards,
}

What do you think?

[main--]

Actually, both of these cases are only concerned with realtime clocks, if I read the docs correctly. So having a read_realtime method that is documented to specifically match the interface of realtime timerfds looks like a really neat solution here.

[petreeftime]

I will look into adding this when I get some spare time, but I'm also seeing occasional panics on ClockId::Monotonic, where it ends up on unreachable!() in read. I think this is either some weird edge case in Linux or some weird occurrence with the underlying hardware, but not sure yet.