Stream: git-wasmtime

Topic: wasmtime / issue #6588 Preview2: Inconsistent Use Of Dead...

Wasmtime GitHub notifications bot (Jun 16 2023 at 12:50):

adamgfraser opened issue #6588:

It appears that there is an inconsistent use of absolute versus relative time in Subscription::MonotonicClock.

Specifically, if you look at how a subscription is constructed here we have:

    pub fn subscribe_monotonic_clock(
        &mut self,
        clock: &'a dyn WasiMonotonicClock,
        deadline: u64,
        absolute: bool,
        ud: Userdata,
    ) {
        let deadline = if absolute {
            // Convert an absolute deadline to a relative one.
            deadline.saturating_sub(clock.now())
        } else {
            deadline
        };
        self.subs.push((
            Subscription::MonotonicClock(MonotonicClockSubscription { clock, deadline }),
            ud,
        ));
    }

This clearly indicates that the deadline is supposed to be a relative duration from the current time, so if we are at t = 2s and want to resume at t = 3s the deadline should be 1s.

However, if you look at how we determine whether a subscription is ready here we do:

    pub fn result(&self) -> Option<Result<(), Error>> {
        if self.now().checked_sub(self.deadline).is_some() {
            Some(Ok(()))
        } else {
            None
        }
    }

Here we are checking if the subscription is ready by looking at whether the current time is after the deadline, assuming that the deadline represents an absolute point in time, which it doesn't as we saw above.

So if in the same example as above we check whether the result is available at t = 2.5s we will get that it is available, since 2.5s is greater than 1s, when it is really not.

Wasmtime GitHub notifications bot (Jun 17 2023 at 01:59):

sunfishcode commented on issue #6588:

Good catch! I've now submitted #6597 to fix this.

Wasmtime GitHub notifications bot (Jun 17 2023 at 02:03):

adamgfraser commented on issue #6588:

Fantastic!

Wasmtime GitHub notifications bot (Jun 21 2023 at 13:29):

sunfishcode closed issue #6588:

It appears that there is an inconsistent use of absolute versus relative time in Subscription::MonotonicClock.

Specifically, if you look at how a subscription is constructed here we have:

    pub fn subscribe_monotonic_clock(
        &mut self,
        clock: &'a dyn WasiMonotonicClock,
        deadline: u64,
        absolute: bool,
        ud: Userdata,
    ) {
        let deadline = if absolute {
            // Convert an absolute deadline to a relative one.
            deadline.saturating_sub(clock.now())
        } else {
            deadline
        };
        self.subs.push((
            Subscription::MonotonicClock(MonotonicClockSubscription { clock, deadline }),
            ud,
        ));
    }

This clearly indicates that the deadline is supposed to be a relative duration from the current time, so if we are at t = 2s and want to resume at t = 3s the deadline should be 1s.

However, if you look at how we determine whether a subscription is ready here we do:

    pub fn result(&self) -> Option<Result<(), Error>> {
        if self.now().checked_sub(self.deadline).is_some() {
            Some(Ok(()))
        } else {
            None
        }
    }

Here we are checking if the subscription is ready by looking at whether the current time is after the deadline, assuming that the deadline represents an absolute point in time, which it doesn't as we saw above.

So if in the same example as above we check whether the result is available at t = 2.5s we will get that it is available, since 2.5s is greater than 1s, when it is really not.

Last updated: Nov 22 2024 at 17:03 UTC