Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

Implement Environment for real tokio runtime #11

Closed
LegNeato opened this issue Dec 16, 2019 · 11 comments
Closed

Implement Environment for real tokio runtime #11

LegNeato opened this issue Dec 16, 2019 · 11 comments

Comments

@LegNeato
Copy link
Contributor

I want to make my functions generic over an Environment. I can do that over the runtimes in this crate but when I pass in the real tokio runtime I cannot due to orphan rules. If I wrap and implement, I get that Environment requires Clone and tokio::runtime::Runtime does not implement it.

I ended up doing this (not sure if I need Mutex or not, I guess not because Runtime is Send and Sync):

/// A runtime that uses real network and clock.
#[derive(Clone)]
pub struct RealRuntime(Arc<tokio::runtime::Runtime>);

#[async_trait]
impl Environment for RealRuntime {
    type TcpStream = tokio::net::TcpStream;
    type TcpListener = tokio::net::TcpListener;

    fn spawn<F>(&self, future: F)
    where
        F: Future<Output = ()> + Send + 'static,
    {
        self.0.clone().spawn(future);
    }
    fn now(&self) -> std::time::Instant {
        std::time::Instant::now()
    }
    fn delay(&self, deadline: std::time::Instant) -> tokio::timer::Delay {
        tokio::timer::delay(deadline)
    }
    fn timeout<T>(&self, value: T, timeout: std::time::Duration) -> tokio::timer::Timeout<T> {
        tokio::timer::Timeout::new(value, timeout)
    }
    async fn bind<A>(&self, addr: A) -> std::io::Result<Self::TcpListener>
    where
        A: Into<std::net::SocketAddr> + Send + Sync,
    {
        tokio::net::TcpListener::bind(&addr.into()).await
    }

    async fn connect<A>(&self, addr: A) -> std::io::Result<Self::TcpStream>
    where
        A: Into<std::net::SocketAddr> + Send + Sync,
    {
        tokio::net::TcpStream::connect(&addr.into()).await
    }
}
@davidbarsky
Copy link
Member

Thanks for noting your approach! Did you happen to see tokio-rs/tokio#1845?

@LegNeato
Copy link
Contributor Author

I did not, thanks for pointing it out!

@LegNeato
Copy link
Contributor Author

LegNeato commented Dec 17, 2019

I talked to @gardnervickers over email, but maybe this whole thing is at the wrong level? Should instead the simulation bits be pushed down into std wherever possible? A benefit of that is we could have standard apis to inspect the simulation and only done if #[cfg(simulation)] or whatever. I was thinking for things like a filesystem and 3rd party crates we may not be able to port and tie to tokio. I also want to check things that use println! and don't see how I would get at that.

@davidbarsky
Copy link
Member

Sorry for the delay: oncall and life are what they are. I think there's a reasonable case to be for pushing the simulation bits into std as much as possible. I'm not sure I know what mechanism would be pursued, but the

  1. In general, generics enable parametrization of code at the type level, while trait objects enable some degree of type-level polymorphism at the cost of being bound to object safety. Perhaps the dynamic trait object approach is fine! I think this is the option that log takes.
  2. Disjoint feature flags that toggle #[cfg(simulation)]. Unfortunately, feature flags are additive, and there's no way to implement a trait on a module (I believe Haskell supports this with Backpack and OCaml through its modules). This would make implementing simulation on other types really nice and easy. I think this RFC 2492 would enable similar behavior, but that RFC has been postponed.
  3. impl Trait usage, where its defining uses are in a struct. I think this is a good approach in the meantime, but I do think a backpack-like solution would be ideal! Unfortunately, this is not supported by Rust at the moment, but it might be in the future: Impl trait in type aliases: defining use via struct rust-lang/rust#66980. Niko and I sketched out an approach that could work, but obviously that isn't implemented yet, but it can be implemented with macros today.

I'd personally love to see simulation work with std in some form, but I have no idea how that would happen, and I don't want to speculate—@gardnervickers probably has a better sense of the future direction.

Few additional notes:

  • I apologize if my comment over-explains some ideas; but I'm not these tradeoffs were ever written down in an issue before.
  • I've got to run now, so this comment might not be 100% clear. Feel free to ask me to clarify anything I said.

@LucioFranco
Copy link
Member

I'm not sure it makes sense for the initial phase of simulation to deal with std. I'd ideally like to see us get interop that works well with tokio's types first.

As for the implementation with the runtime, I'd like to see the choice of the user to choose a simulated type to be explicit but allow the runtime to hook into them.

@davidbarsky
Copy link
Member

As for the implementation with the runtime, I'd like to see the choice of the user to choose a simulated type to be explicit but allow the runtime to hook into them.

Meaning that users would need to pick either TcpStream or SimulatedTcpStream?

@LucioFranco
Copy link
Member

LucioFranco commented Dec 23, 2019

Right, internally, I'd like to see each type use an enum and on creation use a TLS value to check if it should initialize the simulated version or defer to the real one. I think once we standardize things we can move that enumed say TcpStream into tokio, but I'd like to keep the impact of this as minimal as possible for now.

We should be able to add hyper connectors etc in a separate crate to easy the use of it.

@davidbarsky
Copy link
Member

My understanding is that tokio-rs/tokio#1845 does entail the movement of types such as the the sealed/enumerated TcpStream into Tokio. Is that not correct?

@LucioFranco
Copy link
Member

Right, I think we should start though with them living in this crate instead of in tokio. Looks like we need to do a rewrite in this crate anyways right now.

@LucioFranco
Copy link
Member

Its much easier to move fastre in here than it would be in tokio's main repo.

@carllerche
Copy link
Member

Closing as dup of #1845, lets move discussion into there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants