[netstack3] Use zx::Time instead of std::time::Instant for timers
- Previously, we used std::time::Instant because the core's
EventDispatcher trait used that type in order to avoid a platform-
specific dependency on zx::Time; this required some gymnastics,
and meant that we had an open TODO to figure out how to convert
from std::time::Instant to zx::Time
- Now that the EventDispatcher trait provides an abstraction for
the instant at which a timer is scheduled, we can just use zx::Time
directly without exposing that detail to the core, and thus avoid
violating the core's platform-agnosticism requirement
- This also allows us to implement EventDispatcher::schedule_timeout_instant,
which was previously a TODO for the reasons described above
NET-2367 #done
Change-Id: Ic40de7fbaed7ac1f1861cc60f3db14c8937cf743
diff --git a/src/connectivity/network/netstack3/core/src/lib.rs b/src/connectivity/network/netstack3/core/src/lib.rs
index 2242b39..236b92c3 100644
--- a/src/connectivity/network/netstack3/core/src/lib.rs
+++ b/src/connectivity/network/netstack3/core/src/lib.rs
@@ -275,20 +275,27 @@
/// Schedule a callback to be invoked after a timeout.
///
- /// `schedule_timeout` schedules `f` to be invoked after `duration` has elapsed, overwriting any
- /// previous timeout with the same ID.
+ /// `schedule_timeout` schedules `f` to be invoked after `duration` has
+ /// elapsed, overwriting any previous timeout with the same ID.
///
- /// If there was previously a timer with that ID, return the time at which is was scheduled to
- /// fire.
- fn schedule_timeout(&mut self, duration: time::Duration, id: TimerId) -> Option<Self::Instant>;
+ /// If there was previously a timer with that ID, return the time at which
+ /// is was scheduled to fire.
+ ///
+ /// # Panics
+ ///
+ /// `schedule_timeout` may panic if `duration` is large enough that
+ /// `self.now() + duration` overflows.
+ fn schedule_timeout(&mut self, duration: time::Duration, id: TimerId) -> Option<Self::Instant> {
+ self.schedule_timeout_instant(self.now().checked_add(duration).unwrap(), id)
+ }
/// Schedule a callback to be invoked at a specific time.
///
- /// `schedule_timeout_instant` schedules `f` to be invoked at `time`, overwriting any previous
- /// timeout with the same ID.
+ /// `schedule_timeout_instant` schedules `f` to be invoked at `time`,
+ /// overwriting any previous timeout with the same ID.
///
- /// If there was previously a timer with that ID, return the time at which is was scheduled to
- /// fire.
+ /// If there was previously a timer with that ID, return the time at which
+ /// is was scheduled to fire.
fn schedule_timeout_instant(
&mut self,
time: Self::Instant,
diff --git a/src/connectivity/network/netstack3/src/eventloop.rs b/src/connectivity/network/netstack3/src/eventloop.rs
index e96a593..0347875 100644
--- a/src/connectivity/network/netstack3/src/eventloop.rs
+++ b/src/connectivity/network/netstack3/src/eventloop.rs
@@ -77,9 +77,10 @@
use fuchsia_async as fasync;
use fuchsia_zircon as zx;
+use std::convert::TryFrom;
use std::fs::File;
use std::marker::PhantomData;
-use std::time::{Duration, Instant};
+use std::time::Duration;
use failure::{bail, Error};
use fidl::endpoints::{RequestStream, ServiceMarker};
@@ -619,7 +620,7 @@
}
struct TimerInfo {
- time: Instant,
+ time: zx::Time,
id: TimerId,
abort_handle: AbortHandle,
}
@@ -642,26 +643,45 @@
}
}
-impl EventDispatcher for EventLoopInner {
- type Instant = Instant;
+/// A thin wrapper around `zx::Time` that implements `core::Instant`.
+#[derive(Copy, Clone)]
+struct ZxTime(zx::Time);
- fn now(&self) -> Instant {
- Instant::now()
+impl netstack3_core::Instant for ZxTime {
+ fn duration_since(&self, earlier: ZxTime) -> Duration {
+ assert!(self.0 >= earlier.0);
+ // guaranteed not to panic because the assertion ensures that the
+ // difference is non-negative, and all non-negative i64 values are also
+ // valid u64 values
+ Duration::from_nanos(u64::try_from(self.0.into_nanos() - earlier.0.into_nanos()).unwrap())
}
- fn schedule_timeout(&mut self, duration: Duration, id: TimerId) -> Option<Instant> {
- // We need to separately keep track of the time at which the future completes (a Zircon
- // Time object) and the time at which the user expects it to complete. You cannot convert
- // between Zircon Time objects and std::time::Instant objects (since std::time::Instance is
- // opaque), so we generate two different time objects to keep track of.
- let zircon_time = zx::Time::after(zx::Duration::from(duration));
- let rust_time = self.now() + duration;
+ fn checked_add(&self, duration: Duration) -> Option<ZxTime> {
+ Some(ZxTime(zx::Time::from_nanos(
+ self.0.into_nanos().checked_add(i64::try_from(duration.as_nanos()).ok()?)?,
+ )))
+ }
+ fn checked_sub(&self, duration: Duration) -> Option<ZxTime> {
+ Some(ZxTime(zx::Time::from_nanos(
+ self.0.into_nanos().checked_sub(i64::try_from(duration.as_nanos()).ok()?)?,
+ )))
+ }
+}
+
+impl EventDispatcher for EventLoopInner {
+ type Instant = ZxTime;
+
+ fn now(&self) -> ZxTime {
+ ZxTime(zx::Time::get(zx::ClockId::Monotonic))
+ }
+
+ fn schedule_timeout_instant(&mut self, time: ZxTime, id: TimerId) -> Option<ZxTime> {
let old_timer = self.cancel_timeout(id);
let mut timer_send = self.event_send.clone();
let timeout = async move {
- await!(fasync::Timer::new(zircon_time));
+ await!(fasync::Timer::new(time.0));
timer_send.send(Event::TimerEvent(id));
// The timer's cancel function is called by the receiver, so that
// this async block does not need to have a reference to the
@@ -669,7 +689,7 @@
};
let (abort_handle, abort_registration) = AbortHandle::new_pair();
- self.timers.push(TimerInfo { time: rust_time, id, abort_handle });
+ self.timers.push(TimerInfo { time: time.0, id, abort_handle });
let timeout = Abortable::new(timeout, abort_registration);
let timeout = timeout.unwrap_or_else(|_| ());
@@ -678,12 +698,6 @@
old_timer
}
- fn schedule_timeout_instant(&mut self, _time: Instant, _id: TimerId) -> Option<Instant> {
- // It's not possible to convert a std::time::Instant to a Zircon Time, so this API will
- // need some more thought. Punting on this until we need it.
- unimplemented!()
- }
-
// TODO(wesleyac): Fix race
//
// There is a potential race in the following condition:
@@ -699,7 +713,7 @@
// ensuring that the timer future cannot fire until the main queue is empty.
// See `GroupAvailable` in `src/connectivity/wlan/wlanstack/src/future_util.rs`
// for an example of how to do this.
- fn cancel_timeout(&mut self, id: TimerId) -> Option<Instant> {
+ fn cancel_timeout(&mut self, id: TimerId) -> Option<ZxTime> {
let index =
self.timers.iter().enumerate().find_map(
|x| {
@@ -711,7 +725,7 @@
},
)?;
self.timers[index].abort_handle.abort();
- Some(self.timers.remove(index).time)
+ Some(ZxTime(self.timers.remove(index).time))
}
}
