From 5af8a1bf28c544cea4f2e62a163c0e665d5643a2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 14:04:54 -0500 Subject: [PATCH 01/16] Remove misspellings of "rusttls". --- crates/tor-rtcompat/src/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tor-rtcompat/src/test.rs b/crates/tor-rtcompat/src/test.rs index 7c80dc07f..dda653b2f 100644 --- a/crates/tor-rtcompat/src/test.rs +++ b/crates/tor-rtcompat/src/test.rs @@ -266,7 +266,7 @@ macro_rules! tls_runtime_tests { } #[cfg(all(feature="tokio", feature="rustls"))] - mod tokio_rusttls_tests { + mod tokio_rustls_tests { use std::io::Result as IoResult; $( #[test] @@ -276,7 +276,7 @@ macro_rules! tls_runtime_tests { )* } #[cfg(all(feature="async-std", feature="rustls"))] - mod async_std_rusttls_tests { + mod async_std_rustls_tests { use std::io::Result as IoResult; $( #[test] From 884d614a618564d8a80ecb630256e7458c8943f8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 14:06:48 -0500 Subject: [PATCH 02/16] Remove a now-incorrect comment in tor-proto. --- crates/tor-proto/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/tor-proto/src/lib.rs b/crates/tor-proto/src/lib.rs index c20f74986..b863cef68 100644 --- a/crates/tor-proto/src/lib.rs +++ b/crates/tor-proto/src/lib.rs @@ -81,9 +81,6 @@ //! I bet that there are deadlocks somewhere in this code. I fixed //! all the ones I could find or think of, but it would be great to //! find a good way to eliminate every lock that we have. -//! -//! This crate doesn't work with rusttls because of a limitation in the -//! webpki crate. #![deny(missing_docs)] #![warn(noop_method_call)] From 668364d75d5fec06d235a684dfc6c8c89e543503 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 25 Jan 2022 16:52:33 -0500 Subject: [PATCH 03/16] Unify TokioRuntime and TokioRuntimeHandle Having separate types here doesn't justify the (very limited) benefit of distinguishing between the case where we have created an executor that we own and the case where we have a handle to an already-running tokio executor. Part of #301. --- crates/arti-client/examples/hyper.rs | 4 +- crates/arti-client/src/client.rs | 8 +-- crates/tor-rtcompat/src/impls/tokio.rs | 32 ++++++++--- crates/tor-rtcompat/src/tokio.rs | 78 +++++++++----------------- 4 files changed, 55 insertions(+), 67 deletions(-) diff --git a/crates/arti-client/examples/hyper.rs b/crates/arti-client/examples/hyper.rs index 411ac572a..26494231f 100644 --- a/crates/arti-client/examples/hyper.rs +++ b/crates/arti-client/examples/hyper.rs @@ -13,7 +13,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; use tokio_crate as tokio; -use tor_rtcompat::tokio::TokioRuntimeHandle; +use tor_rtcompat::tokio::TokioRuntime; use tor_rtcompat::Runtime; /// A `hyper` connector to proxy HTTP connections via the Tor network, using Arti. @@ -138,7 +138,7 @@ async fn main() -> Result<()> { // on Linux platforms) let config = TorClientConfig::default(); // Arti needs an async runtime handle to spawn async tasks. - let rt: TokioRuntimeHandle = tokio_crate::runtime::Handle::current().into(); + let rt: TokioRuntime = tokio_crate::runtime::Handle::current().into(); // We now let the Arti client start and bootstrap a connection to the network. // (This takes a while to gather the necessary consensus state, etc.) diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index ce019e24b..eb03bb1ef 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -26,7 +26,7 @@ use crate::{status, Error, Result}; #[cfg(feature = "async-std")] use tor_rtcompat::async_std::AsyncStdRuntime; #[cfg(feature = "tokio")] -use tor_rtcompat::tokio::TokioRuntimeHandle; +use tor_rtcompat::tokio::TokioRuntime; use tracing::{debug, error, info, warn}; /// An active client session on the Tor network. @@ -246,7 +246,7 @@ impl StreamPrefs { } #[cfg(feature = "tokio")] -impl TorClient { +impl TorClient { /// Bootstrap a connection to the Tor network, using the current Tokio runtime. /// /// Returns a client once there is enough directory material to @@ -257,9 +257,7 @@ impl TorClient { /// # Panics /// /// Panics if called outside of the context of a Tokio runtime. - pub async fn bootstrap_with_tokio( - config: TorClientConfig, - ) -> Result> { + pub async fn bootstrap_with_tokio(config: TorClientConfig) -> Result> { let rt = tor_rtcompat::tokio::current_runtime().expect("called outside of Tokio runtime"); Self::bootstrap(rt, config).await } diff --git a/crates/tor-rtcompat/src/impls/tokio.rs b/crates/tor-rtcompat/src/impls/tokio.rs index bde869e9f..fc2d5da42 100644 --- a/crates/tor-rtcompat/src/impls/tokio.rs +++ b/crates/tor-rtcompat/src/impls/tokio.rs @@ -154,10 +154,11 @@ macro_rules! implement_traits_for { } /// Create and return a new Tokio multithreaded runtime. -pub(crate) fn create_runtime() -> IoResult { +pub(crate) fn create_runtime() -> IoResult { let mut builder = async_executors::TokioTpBuilder::new(); builder.tokio_builder().enable_all(); - builder.build() + let owned = builder.build()?; + Ok(owned.into()) } /// Wrapper around a Handle to a tokio runtime. @@ -174,6 +175,12 @@ pub(crate) fn create_runtime() -> IoResult { /// that when creating this object. #[derive(Clone, Debug)] pub struct TokioRuntimeHandle { + /// If present, the tokio executor that we've created (and which we own). + /// + /// We never access this directly; only through `handle`. We keep it here + /// so that our Runtime types can be agnostic about whether they own the + /// executor. + owned: Option, /// The underlying Handle. handle: tokio_crate::runtime::Handle, } @@ -189,17 +196,29 @@ impl TokioRuntimeHandle { pub(crate) fn new(handle: tokio_crate::runtime::Handle) -> Self { handle.into() } + + /// Return true if this handle owns the executor that it points to. + pub fn is_owned(&self) -> bool { + self.owned.is_some() + } } impl From for TokioRuntimeHandle { fn from(handle: tokio_crate::runtime::Handle) -> Self { - Self { handle } + Self { + owned: None, + handle, + } } } -impl SpawnBlocking for async_executors::TokioTp { - fn block_on(&self, f: F) -> F::Output { - async_executors::TokioTp::block_on(self, f) +impl From for TokioRuntimeHandle { + fn from(owner: async_executors::TokioTp) -> TokioRuntimeHandle { + let handle = owner.block_on(async { tokio_crate::runtime::Handle::current() }); + Self { + owned: Some(owner), + handle, + } } } @@ -220,5 +239,4 @@ impl futures::task::Spawn for TokioRuntimeHandle { } } -implement_traits_for! {async_executors::TokioTp} implement_traits_for! {TokioRuntimeHandle} diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index cc0229a52..63b238ee6 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -1,7 +1,6 @@ //! Entry points for use with Tokio runtimes. use crate::impls::native_tls::NativeTlsProvider; use crate::impls::tokio::TokioRuntimeHandle as Handle; -use async_executors::TokioTp; use crate::{CompoundRuntime, Runtime, SpawnBlocking}; use std::io::{Error as IoError, ErrorKind, Result as IoResult}; @@ -18,7 +17,7 @@ use crate::impls::tokio::net::TcpStream; /// implementations for Tokio's time, net, and io facilities, but we have /// no good way to check that when creating this object. #[derive(Clone)] -pub struct TokioRuntimeHandle { +pub struct TokioRuntime { /// The actual [`CompoundRuntime`] that implements this. inner: HandleInner, } @@ -29,7 +28,7 @@ type HandleInner = CompoundRuntime>; -/// A [`Runtime`] built around an owned `TokioTp` executor, and `native_tls`. -#[derive(Clone)] -pub struct TokioRuntime { - /// The actual [`CompoundRuntime`] that implements this. - inner: TokioRuntimeInner, -} - -/// A [`Runtime`] built around an owned `TokioTp` executor, and `rustls`. -#[derive(Clone)] -#[cfg(feature = "rustls")] -pub struct TokioRustlsRuntime { - /// The actual [`CompoundRuntime`] that implements this. - inner: TokioRustlsRuntimeInner, -} - -/// Implementation type for TokioRuntime. -type TokioRuntimeInner = CompoundRuntime>; - -/// Implementation type for TokioRustlsRuntime. -#[cfg(feature = "rustls")] -type TokioRustlsRuntimeInner = - CompoundRuntime>; - crate::opaque::implement_opaque_runtime! { - TokioRuntimeHandle { inner : HandleInner } -} - -crate::opaque::implement_opaque_runtime! { - TokioRuntime { inner : TokioRuntimeInner } + TokioRuntime { inner : HandleInner } } #[cfg(feature = "rustls")] crate::opaque::implement_opaque_runtime! { - TokioRustlsRuntimeHandle { inner : RustlsHandleInner } + TokioRustlsRuntime { inner : RustlsHandleInner } } -#[cfg(feature = "rustls")] -crate::opaque::implement_opaque_runtime! { - TokioRustlsRuntime { inner : TokioRustlsRuntimeInner } -} - -impl From for TokioRuntimeHandle { +impl From for TokioRuntime { fn from(h: tokio_crate::runtime::Handle) -> Self { let h = Handle::new(h); - TokioRuntimeHandle { + TokioRuntime { inner: CompoundRuntime::new(h.clone(), h.clone(), h, NativeTlsProvider::default()), } } } +#[cfg(feature = "rustls")] +impl From for TokioRustlsRuntime { + fn from(h: tokio_crate::runtime::Handle) -> Self { + let h = Handle::new(h); + TokioRustlsRuntime { + inner: CompoundRuntime::new(h.clone(), h.clone(), h, RustlsProvider::default()), + } + } +} + /// Create and return a new Tokio multithreaded runtime. fn create_tokio_runtime() -> IoResult { crate::impls::tokio::create_runtime().map(|r| TokioRuntime { @@ -144,25 +121,20 @@ pub fn create_rustls_runtime() -> std::io::Result { /// /// Once you have a runtime returned by this function, you should /// just create more handles to it via [`Clone`]. -pub fn current_runtime() -> std::io::Result { - let handle = tokio_crate::runtime::Handle::try_current() - .map_err(|e| IoError::new(ErrorKind::Other, e))?; - let h = Handle::new(handle); - Ok(TokioRuntimeHandle { - inner: CompoundRuntime::new(h.clone(), h.clone(), h, NativeTlsProvider::default()), - }) +pub fn current_runtime() -> std::io::Result { + Ok(current_handle()?.into()) } /// Return an instance of the currently running tokio [`Runtime`], wrapped to /// use `rustls`. #[cfg(feature = "rustls")] -pub fn current_runtime_rustls() -> std::io::Result { - let handle = tokio_crate::runtime::Handle::try_current() - .map_err(|e| IoError::new(ErrorKind::Other, e))?; - let h = Handle::new(handle); - Ok(TokioRustlsRuntimeHandle { - inner: CompoundRuntime::new(h.clone(), h.clone(), h, RustlsProvider::default()), - }) +pub fn current_runtime_rustls() -> std::io::Result { + Ok(current_handle()?.into()) +} + +/// As `Handle::try_current()`, but return an IoError on failure. +fn current_handle() -> std::io::Result { + tokio_crate::runtime::Handle::try_current().map_err(|e| IoError::new(ErrorKind::Other, e)) } /// Run a test function using a freshly created tokio runtime. From 99c59a8f2b3d1d97110ecdfc1021668f88177c85 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 25 Jan 2022 17:58:44 -0500 Subject: [PATCH 04/16] Remove no-longer-needed tokio runtime helper macro --- crates/tor-rtcompat/src/impls/tokio.rs | 45 +++++++++++--------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/crates/tor-rtcompat/src/impls/tokio.rs b/crates/tor-rtcompat/src/impls/tokio.rs index fc2d5da42..85f207617 100644 --- a/crates/tor-rtcompat/src/impls/tokio.rs +++ b/crates/tor-rtcompat/src/impls/tokio.rs @@ -124,33 +124,26 @@ use futures::Future; use std::io::Result as IoResult; use std::time::Duration; -/// Helper: Declare that a given tokio runtime object implements the -/// prerequisites for Runtime. -// TODO: Maybe we can do this more simply with a simpler trait? -macro_rules! implement_traits_for { - ($runtime:ty) => { - impl SleepProvider for $runtime { - type SleepFuture = tokio_crate::time::Sleep; - fn sleep(&self, duration: Duration) -> Self::SleepFuture { - tokio_crate::time::sleep(duration) - } - } +impl SleepProvider for TokioRuntimeHandle { + type SleepFuture = tokio_crate::time::Sleep; + fn sleep(&self, duration: Duration) -> Self::SleepFuture { + tokio_crate::time::sleep(duration) + } +} - #[async_trait] - impl crate::traits::TcpProvider for $runtime { - type TcpStream = net::TcpStream; - type TcpListener = net::TcpListener; +#[async_trait] +impl crate::traits::TcpProvider for TokioRuntimeHandle { + type TcpStream = net::TcpStream; + type TcpListener = net::TcpListener; - async fn connect(&self, addr: &std::net::SocketAddr) -> IoResult { - let s = net::TokioTcpStream::connect(addr).await?; - Ok(s.into()) - } - async fn listen(&self, addr: &std::net::SocketAddr) -> IoResult { - let lis = net::TokioTcpListener::bind(*addr).await?; - Ok(net::TcpListener { lis }) - } - } - }; + async fn connect(&self, addr: &std::net::SocketAddr) -> IoResult { + let s = net::TokioTcpStream::connect(addr).await?; + Ok(s.into()) + } + async fn listen(&self, addr: &std::net::SocketAddr) -> IoResult { + let lis = net::TokioTcpListener::bind(*addr).await?; + Ok(net::TcpListener { lis }) + } } /// Create and return a new Tokio multithreaded runtime. @@ -238,5 +231,3 @@ impl futures::task::Spawn for TokioRuntimeHandle { Ok(()) } } - -implement_traits_for! {TokioRuntimeHandle} From 05a04220ccc4d0dcbab906db42abc97bad02f222 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 25 Jan 2022 17:22:14 -0500 Subject: [PATCH 05/16] Limit the inner types in tor-rtcompat that have to implement Clone If we implement our own clone on CompoundRuntime, we no longer need Clone implementations on our TlsProvider implementations. --- crates/tor-rtcompat/src/compound.rs | 11 ++++++++++- crates/tor-rtcompat/src/impls/native_tls.rs | 10 ---------- crates/tor-rtcompat/src/impls/rustls.rs | 22 --------------------- 3 files changed, 10 insertions(+), 33 deletions(-) diff --git a/crates/tor-rtcompat/src/compound.rs b/crates/tor-rtcompat/src/compound.rs index f5bd94ab4..8df5157f3 100644 --- a/crates/tor-rtcompat/src/compound.rs +++ b/crates/tor-rtcompat/src/compound.rs @@ -18,7 +18,6 @@ use std::io::Result as IoResult; /// You can use this structure to create new runtimes in two ways: either by /// overriding a single part of an existing runtime, or by building an entirely /// new runtime from pieces. -#[derive(Clone)] pub struct CompoundRuntime { /// The actual collection of Runtime objects. /// @@ -27,6 +26,16 @@ pub struct CompoundRuntime { inner: Arc>, } +// We have to provide this ourselves, since derive(Clone) wrongly infers a +// `where S: Clone` bound (from the generic argument). +impl Clone for CompoundRuntime { + fn clone(&self) -> Self { + Self { + inner: Arc::clone(&self.inner), + } + } +} + /// A collection of objects implementing that traits that make up a [`Runtime`] struct Inner { /// A `Spawn` and `SpawnBlocking` implementation. diff --git a/crates/tor-rtcompat/src/impls/native_tls.rs b/crates/tor-rtcompat/src/impls/native_tls.rs index 193e551cc..a0de354da 100644 --- a/crates/tor-rtcompat/src/impls/native_tls.rs +++ b/crates/tor-rtcompat/src/impls/native_tls.rs @@ -103,13 +103,3 @@ impl Default for NativeTlsProvider { Self::new() } } - -// We have to provide this ourselves, since derive(Clone) wrongly infers a -// `where S: Clone` bound (from the generic argument). -impl Clone for NativeTlsProvider { - fn clone(&self) -> Self { - Self { - _phantom: std::marker::PhantomData, - } - } -} diff --git a/crates/tor-rtcompat/src/impls/rustls.rs b/crates/tor-rtcompat/src/impls/rustls.rs index 1cc482c65..93f300d84 100644 --- a/crates/tor-rtcompat/src/impls/rustls.rs +++ b/crates/tor-rtcompat/src/impls/rustls.rs @@ -100,28 +100,6 @@ impl Default for RustlsProvider { } } -// We have to provide this ourselves, since derive(Clone) wrongly infers a -// `where S: Clone` bound (from the generic argument). -impl Clone for RustlsProvider { - fn clone(&self) -> Self { - Self { - config: Arc::clone(&self.config), - _phantom: std::marker::PhantomData, - } - } -} - -// We have to provide this ourselves, since derive(Clone) wrongly infers a -// `where S: Clone` bound (from the generic argument). -impl Clone for RustlsConnector { - fn clone(&self) -> Self { - Self { - connector: self.connector.clone(), - _phantom: std::marker::PhantomData, - } - } -} - /// A [`rustls::ServerCertVerifier`] based on the [`x509_signature`] crate. /// /// This verifier is necessary since Tor relays doesn't participate in the web From 2333d0466e482810dbe91a6c17f6b363d7956f36 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 08:16:39 -0500 Subject: [PATCH 06/16] Rename FooRuntime to FooNativeTlsRuntime for consistency. --- crates/arti-client/examples/hyper.rs | 4 ++-- crates/arti-client/src/client.rs | 14 ++++++++------ crates/tor-circmgr/src/lib.rs | 2 +- crates/tor-rtcompat/src/async_std.rs | 12 ++++++------ crates/tor-rtcompat/src/tokio.rs | 16 ++++++++-------- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/crates/arti-client/examples/hyper.rs b/crates/arti-client/examples/hyper.rs index 26494231f..b589cf6a1 100644 --- a/crates/arti-client/examples/hyper.rs +++ b/crates/arti-client/examples/hyper.rs @@ -13,7 +13,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; use tokio_crate as tokio; -use tor_rtcompat::tokio::TokioRuntime; +use tor_rtcompat::tokio::TokioNativeTlsRuntime; use tor_rtcompat::Runtime; /// A `hyper` connector to proxy HTTP connections via the Tor network, using Arti. @@ -138,7 +138,7 @@ async fn main() -> Result<()> { // on Linux platforms) let config = TorClientConfig::default(); // Arti needs an async runtime handle to spawn async tasks. - let rt: TokioRuntime = tokio_crate::runtime::Handle::current().into(); + let rt: TokioNativeTlsRuntime = tokio_crate::runtime::Handle::current().into(); // We now let the Arti client start and bootstrap a connection to the network. // (This takes a while to gather the necessary consensus state, etc.) diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index eb03bb1ef..75527627d 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -24,9 +24,9 @@ use std::time::Duration; use crate::{status, Error, Result}; #[cfg(feature = "async-std")] -use tor_rtcompat::async_std::AsyncStdRuntime; +use tor_rtcompat::async_std::AsyncStdNativeTlsRuntime; #[cfg(feature = "tokio")] -use tor_rtcompat::tokio::TokioRuntime; +use tor_rtcompat::tokio::TokioNativeTlsRuntime; use tracing::{debug, error, info, warn}; /// An active client session on the Tor network. @@ -246,7 +246,7 @@ impl StreamPrefs { } #[cfg(feature = "tokio")] -impl TorClient { +impl TorClient { /// Bootstrap a connection to the Tor network, using the current Tokio runtime. /// /// Returns a client once there is enough directory material to @@ -257,14 +257,16 @@ impl TorClient { /// # Panics /// /// Panics if called outside of the context of a Tokio runtime. - pub async fn bootstrap_with_tokio(config: TorClientConfig) -> Result> { + pub async fn bootstrap_with_tokio( + config: TorClientConfig, + ) -> Result> { let rt = tor_rtcompat::tokio::current_runtime().expect("called outside of Tokio runtime"); Self::bootstrap(rt, config).await } } #[cfg(feature = "async-std")] -impl TorClient { +impl TorClient { /// Bootstrap a connection to the Tor network, using the current async-std runtime. /// /// Returns a client once there is enough directory material to @@ -273,7 +275,7 @@ impl TorClient { /// This is a convenience wrapper around [`TorClient::bootstrap`]. pub async fn bootstrap_with_async_std( config: TorClientConfig, - ) -> Result> { + ) -> Result> { // FIXME(eta): not actually possible for this to fail let rt = tor_rtcompat::async_std::current_runtime().expect("failed to get async-std runtime"); diff --git a/crates/tor-circmgr/src/lib.rs b/crates/tor-circmgr/src/lib.rs index fdc6501b5..01767a34c 100644 --- a/crates/tor-circmgr/src/lib.rs +++ b/crates/tor-circmgr/src/lib.rs @@ -440,7 +440,7 @@ mod test { /// Helper type used to help type inference. pub(crate) type OptDummyGuardMgr<'a> = - Option<&'a tor_guardmgr::GuardMgr>; + Option<&'a tor_guardmgr::GuardMgr>; #[test] fn get_params() { diff --git a/crates/tor-rtcompat/src/async_std.rs b/crates/tor-rtcompat/src/async_std.rs index d0d83eb42..7cf768a72 100644 --- a/crates/tor-rtcompat/src/async_std.rs +++ b/crates/tor-rtcompat/src/async_std.rs @@ -12,7 +12,7 @@ use async_executors::AsyncStd; /// A [`Runtime`](crate::Runtime) powered by `async_std` and `native_tls`. #[derive(Clone)] -pub struct AsyncStdRuntime { +pub struct AsyncStdNativeTlsRuntime { /// The actual runtime object. inner: NativeTlsInner, } @@ -21,7 +21,7 @@ pub struct AsyncStdRuntime { type NativeTlsInner = CompoundRuntime>; crate::opaque::implement_opaque_runtime! { - AsyncStdRuntime { inner : NativeTlsInner } + AsyncStdNativeTlsRuntime { inner : NativeTlsInner } } #[cfg(feature = "rustls")] @@ -46,9 +46,9 @@ crate::opaque::implement_opaque_runtime! { /// Generally you should call this function only once, and then use /// [`Clone::clone()`] to create additional references to that /// runtime. -pub fn create_runtime() -> std::io::Result { +pub fn create_runtime() -> std::io::Result { let rt = create_runtime_impl(); - Ok(AsyncStdRuntime { + Ok(AsyncStdNativeTlsRuntime { inner: CompoundRuntime::new(rt, rt, rt, NativeTlsProvider::default()), }) } @@ -64,7 +64,7 @@ pub fn create_rustls_runtime() -> std::io::Result { /// Try to return an instance of the currently running async_std /// [`Runtime`](crate::Runtime). -pub fn current_runtime() -> std::io::Result { +pub fn current_runtime() -> std::io::Result { // In async_std, the runtime is a global singleton. create_runtime() } @@ -72,7 +72,7 @@ pub fn current_runtime() -> std::io::Result { /// Run a test function using a freshly created async_std runtime. pub fn test_with_runtime(func: P) -> O where - P: FnOnce(AsyncStdRuntime) -> F, + P: FnOnce(AsyncStdNativeTlsRuntime) -> F, F: futures::Future, { let runtime = current_runtime().expect("Couldn't get global async_std runtime?"); diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index 63b238ee6..8434421ed 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -17,7 +17,7 @@ use crate::impls::tokio::net::TcpStream; /// implementations for Tokio's time, net, and io facilities, but we have /// no good way to check that when creating this object. #[derive(Clone)] -pub struct TokioRuntime { +pub struct TokioNativeTlsRuntime { /// The actual [`CompoundRuntime`] that implements this. inner: HandleInner, } @@ -38,7 +38,7 @@ pub struct TokioRustlsRuntime { type RustlsHandleInner = CompoundRuntime>; crate::opaque::implement_opaque_runtime! { - TokioRuntime { inner : HandleInner } + TokioNativeTlsRuntime { inner : HandleInner } } #[cfg(feature = "rustls")] @@ -46,10 +46,10 @@ crate::opaque::implement_opaque_runtime! { TokioRustlsRuntime { inner : RustlsHandleInner } } -impl From for TokioRuntime { +impl From for TokioNativeTlsRuntime { fn from(h: tokio_crate::runtime::Handle) -> Self { let h = Handle::new(h); - TokioRuntime { + TokioNativeTlsRuntime { inner: CompoundRuntime::new(h.clone(), h.clone(), h, NativeTlsProvider::default()), } } @@ -66,8 +66,8 @@ impl From for TokioRustlsRuntime { } /// Create and return a new Tokio multithreaded runtime. -fn create_tokio_runtime() -> IoResult { - crate::impls::tokio::create_runtime().map(|r| TokioRuntime { +fn create_tokio_runtime() -> IoResult { + crate::impls::tokio::create_runtime().map(|r| TokioNativeTlsRuntime { inner: CompoundRuntime::new(r.clone(), r.clone(), r, NativeTlsProvider::default()), }) } @@ -121,7 +121,7 @@ pub fn create_rustls_runtime() -> std::io::Result { /// /// Once you have a runtime returned by this function, you should /// just create more handles to it via [`Clone`]. -pub fn current_runtime() -> std::io::Result { +pub fn current_runtime() -> std::io::Result { Ok(current_handle()?.into()) } @@ -144,7 +144,7 @@ fn current_handle() -> std::io::Result { /// Panics if we can't create a tokio runtime. pub fn test_with_runtime(func: P) -> O where - P: FnOnce(TokioRuntime) -> F, + P: FnOnce(TokioNativeTlsRuntime) -> F, F: futures::Future, { let runtime = create_tokio_runtime().expect("Failed to create a tokio runtime"); From 6f29d485e460b30cba992bcb407bf3c7e2a479b7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 08:40:35 -0500 Subject: [PATCH 07/16] Make current/create functions into runtime member functions. This should help avoid some amount of temptation towards API proliferation. --- crates/arti-bench/src/main.rs | 2 +- crates/arti-client/examples/readme.rs | 3 +- crates/arti-client/src/client.rs | 5 +- crates/arti-client/src/lib.rs | 2 +- crates/arti/src/main.rs | 8 +- crates/tor-rtcompat/src/async_std.rs | 69 +++++++++----- crates/tor-rtcompat/src/lib.rs | 8 +- crates/tor-rtcompat/src/test.rs | 12 +-- crates/tor-rtcompat/src/tokio.rs | 124 +++++++++++++------------- 9 files changed, 128 insertions(+), 105 deletions(-) diff --git a/crates/arti-bench/src/main.rs b/crates/arti-bench/src/main.rs index 79009bfa8..fae677ccd 100644 --- a/crates/arti-bench/src/main.rs +++ b/crates/arti-bench/src/main.rs @@ -371,7 +371,7 @@ fn main() -> Result<()> { concurrent: parallel, upload_payload, download_payload, - runtime: tor_rtcompat::tokio::create_runtime()?, + runtime: tor_rtcompat::tokio::TokioNativeTlsRuntime::create()?, results: Default::default(), }; diff --git a/crates/arti-client/examples/readme.rs b/crates/arti-client/examples/readme.rs index 07b7ce21d..4fdbbf829 100644 --- a/crates/arti-client/examples/readme.rs +++ b/crates/arti-client/examples/readme.rs @@ -1,6 +1,7 @@ use anyhow::Result; use arti_client::{TorClient, TorClientConfig}; use tokio_crate as tokio; +use tor_rtcompat::tokio::TokioNativeTlsRuntime; use futures::io::{AsyncReadExt, AsyncWriteExt}; @@ -16,7 +17,7 @@ async fn main() -> Result<()> { let config = TorClientConfig::default(); // Arti needs an async runtime handle to spawn async tasks. // (See "Multiple runtime support" below.) - let rt = tor_rtcompat::tokio::current_runtime()?; + let rt = TokioNativeTlsRuntime::current()?; eprintln!("connecting to Tor..."); diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index 75527627d..e1e634b2f 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -260,7 +260,7 @@ impl TorClient { pub async fn bootstrap_with_tokio( config: TorClientConfig, ) -> Result> { - let rt = tor_rtcompat::tokio::current_runtime().expect("called outside of Tokio runtime"); + let rt = TokioNativeTlsRuntime::current().expect("called outside of Tokio runtime"); Self::bootstrap(rt, config).await } } @@ -277,8 +277,7 @@ impl TorClient { config: TorClientConfig, ) -> Result> { // FIXME(eta): not actually possible for this to fail - let rt = - tor_rtcompat::async_std::current_runtime().expect("failed to get async-std runtime"); + let rt = AsyncStdNativeTlsRuntime::current().expect("failed to get async-std runtime"); Self::bootstrap(rt, config).await } } diff --git a/crates/arti-client/src/lib.rs b/crates/arti-client/src/lib.rs index b3caab7fc..69e3cf578 100644 --- a/crates/arti-client/src/lib.rs +++ b/crates/arti-client/src/lib.rs @@ -60,7 +60,7 @@ //! let config = TorClientConfig::default(); //! // Arti needs a handle to an async runtime in order to spawn tasks and use the //! // network. (See "Multiple runtime support" below.) -//! let rt = tor_rtcompat::tokio::current_runtime()?; +//! let rt = tor_rtcompat::tokio::TokioNativeTlsRuntime::current()?; //! //! // Start the Arti client, and let it bootstrap a connection to the Tor network. //! // (This takes a while to gather the necessary directory information. diff --git a/crates/arti/src/main.rs b/crates/arti/src/main.rs index 4df079e3f..0a5a33ce2 100644 --- a/crates/arti/src/main.rs +++ b/crates/arti/src/main.rs @@ -229,10 +229,12 @@ fn main() -> Result<()> { process::use_max_file_limit(); - #[cfg(feature = "tokio")] - let runtime = tor_rtcompat::tokio::create_runtime()?; #[cfg(all(feature = "async-std", not(feature = "tokio")))] - let runtime = tor_rtcompat::async_std::create_runtime()?; + use tor_rtcompat::tokio::AsyncStdNativeTlsRuntime as ChosenRuntime; + #[cfg(feature = "tokio")] + use tor_rtcompat::tokio::TokioNativeTlsRuntime as ChosenRuntime; + + let runtime = ChosenRuntime::create()?; let rt_copy = runtime.clone(); rt_copy.block_on(run(runtime, socks_port, client_config))?; diff --git a/crates/tor-rtcompat/src/async_std.rs b/crates/tor-rtcompat/src/async_std.rs index 7cf768a72..e672b133f 100644 --- a/crates/tor-rtcompat/src/async_std.rs +++ b/crates/tor-rtcompat/src/async_std.rs @@ -1,6 +1,7 @@ //! Entry points for use with async_std runtimes. pub use crate::impls::async_std::create_runtime as create_runtime_impl; use crate::{compound::CompoundRuntime, SpawnBlocking}; +use std::io::Result as IoResult; use crate::impls::native_tls::NativeTlsProvider; @@ -41,32 +42,53 @@ crate::opaque::implement_opaque_runtime! { AsyncStdRustlsRuntime { inner: RustlsInner } } -/// Return a new async-std-based [`Runtime`](crate::Runtime). -/// -/// Generally you should call this function only once, and then use -/// [`Clone::clone()`] to create additional references to that -/// runtime. -pub fn create_runtime() -> std::io::Result { - let rt = create_runtime_impl(); - Ok(AsyncStdNativeTlsRuntime { - inner: CompoundRuntime::new(rt, rt, rt, NativeTlsProvider::default()), - }) +impl AsyncStdNativeTlsRuntime { + /// Return a new [`AsyncStdNativeTlsRuntime`] + /// + /// Generally you should call this function only once, and then use + /// [`Clone::clone()`] to create additional references to that + /// runtime. + pub fn create() -> IoResult { + let rt = create_runtime_impl(); + Ok(AsyncStdNativeTlsRuntime { + inner: CompoundRuntime::new(rt, rt, rt, NativeTlsProvider::default()), + }) + } + + /// Return an [`AsyncStdNativeTlsRuntime`] for the currently running + /// `async_std` executor. + /// + /// Note that since async_std executors are global, there is no distinction + /// between this method and [`AsyncStdNativeTlsRuntime::create()`]: it is + /// provided only for API consistency with the Tokio runtimes. + pub fn current() -> IoResult { + Self::create() + } } -/// Return a new [`Runtime`](crate::Runtime) based on `async_std` and `rustls`. #[cfg(feature = "rustls")] -pub fn create_rustls_runtime() -> std::io::Result { - let rt = create_runtime_impl(); - Ok(AsyncStdRustlsRuntime { - inner: CompoundRuntime::new(rt, rt, rt, RustlsProvider::default()), - }) -} +impl AsyncStdRustlsRuntime { + /// Return a new [`AsyncStdRustlsRuntime`] + /// + /// Generally you should call this function only once, and then use + /// [`Clone::clone()`] to create additional references to that + /// runtime. + pub fn create() -> IoResult { + let rt = create_runtime_impl(); + Ok(AsyncStdRustlsRuntime { + inner: CompoundRuntime::new(rt, rt, rt, RustlsProvider::default()), + }) + } -/// Try to return an instance of the currently running async_std -/// [`Runtime`](crate::Runtime). -pub fn current_runtime() -> std::io::Result { - // In async_std, the runtime is a global singleton. - create_runtime() + /// Return an [`AsyncStdRustlsRuntime`] for the currently running + /// `async_std` executor. + /// + /// Note that since async_std executors are global, there is no distinction + /// between this method and [`AsyncStdNativeTlsRuntime::create()`]: it is + /// provided only for API consistency with the Tokio runtimes. + pub fn current() -> IoResult { + Self::create() + } } /// Run a test function using a freshly created async_std runtime. @@ -75,6 +97,7 @@ where P: FnOnce(AsyncStdNativeTlsRuntime) -> F, F: futures::Future, { - let runtime = current_runtime().expect("Couldn't get global async_std runtime?"); + let runtime = + AsyncStdNativeTlsRuntime::create().expect("Couldn't get global async_std runtime?"); runtime.clone().block_on(func(runtime)) } diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index da7ca44d2..7ae00362b 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -192,11 +192,11 @@ pub use compound::CompoundRuntime; pub fn current_user_runtime() -> std::io::Result { #[cfg(feature = "tokio")] { - crate::tokio::current_runtime() + crate::tokio::TokioNativeTlsRuntime::current() } #[cfg(all(feature = "async-std", not(feature = "tokio")))] { - crate::async_std::current_runtime() + crate::async_std::AsyncStdNativeTlsRuntime::current() } } @@ -218,11 +218,11 @@ pub fn current_user_runtime() -> std::io::Result { pub fn create_runtime() -> std::io::Result { #[cfg(feature = "tokio")] { - crate::tokio::create_runtime() + crate::tokio::TokioNativeTlsRuntime::create() } #[cfg(all(feature = "async-std", not(feature = "tokio")))] { - crate::async_std::create_runtime() + crate::async_std::AsyncStdNativeTlsRuntime::create() } } diff --git a/crates/tor-rtcompat/src/test.rs b/crates/tor-rtcompat/src/test.rs index dda653b2f..40dbe3b77 100644 --- a/crates/tor-rtcompat/src/test.rs +++ b/crates/tor-rtcompat/src/test.rs @@ -225,7 +225,7 @@ macro_rules! runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::tokio::create_runtime()?) + super::$id(&crate::tokio::TokioNativeTlsRuntime::create()?) } )* } @@ -235,7 +235,7 @@ macro_rules! runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::async_std::create_runtime()?) + super::$id(&crate::async_std::AsyncStdNativeTlsRuntime::create()?) } )* } @@ -250,7 +250,7 @@ macro_rules! tls_runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::tokio::create_runtime()?) + super::$id(&crate::tokio::TokioNativeTlsRuntime::create()?) } )* } @@ -260,7 +260,7 @@ macro_rules! tls_runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::async_std::create_runtime()?) + super::$id(&crate::async_std::AsyncStdNativeTlsRuntime::create()?) } )* } @@ -271,7 +271,7 @@ macro_rules! tls_runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::tokio::create_rustls_runtime()?) + super::$id(&crate::tokio::TokioRustlsRuntime::create()?) } )* } @@ -281,7 +281,7 @@ macro_rules! tls_runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::async_std::create_rustls_runtime()?) + super::$id(&crate::async_std::AsyncStdRustlsRuntime::create()?) } )* } diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index 8434421ed..a2f680e98 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -2,7 +2,7 @@ use crate::impls::native_tls::NativeTlsProvider; use crate::impls::tokio::TokioRuntimeHandle as Handle; -use crate::{CompoundRuntime, Runtime, SpawnBlocking}; +use crate::{CompoundRuntime, SpawnBlocking}; use std::io::{Error as IoError, ErrorKind, Result as IoResult}; #[cfg(feature = "rustls")] @@ -65,71 +65,69 @@ impl From for TokioRustlsRuntime { } } -/// Create and return a new Tokio multithreaded runtime. -fn create_tokio_runtime() -> IoResult { - crate::impls::tokio::create_runtime().map(|r| TokioNativeTlsRuntime { - inner: CompoundRuntime::new(r.clone(), r.clone(), r, NativeTlsProvider::default()), - }) +impl TokioNativeTlsRuntime { + /// Create a new [`TokioNativeTlsRuntime`]. + /// + /// The return value will own the underlying Tokio runtime object, which + /// will be dropped when the last copy of this handle is freed. + /// + /// If you want to use a currently running runtime instead, call + /// [`TokioNativeTlsRuntime::current()`]. + pub fn create() -> IoResult { + crate::impls::tokio::create_runtime().map(|r| TokioNativeTlsRuntime { + inner: CompoundRuntime::new(r.clone(), r.clone(), r, NativeTlsProvider::default()), + }) + } + + /// Return a [`TokioNativeTlsRuntime`] wrapping the currently running + /// Tokio runtime. + /// + /// # Usage note + /// + /// We should never call this from inside other Arti crates, or from library + /// crates that want to support multiple runtimes! This function is for + /// Arti _users_ who want to wrap some existing Tokio runtime as a + /// [`Runtime`]. It is not for library crates that want to work with + /// multiple runtimes. + /// + /// Once you have a runtime returned by this function, you should just + /// create more handles to it via [`Clone`]. + pub fn current() -> IoResult { + Ok(current_handle()?.into()) + } } -/// Create and return a new Tokio multithreaded runtime configured to use `rustls`. #[cfg(feature = "rustls")] -fn create_tokio_rustls_runtime() -> IoResult { - crate::impls::tokio::create_runtime().map(|r| TokioRustlsRuntime { - inner: CompoundRuntime::new(r.clone(), r.clone(), r, RustlsProvider::default()), - }) -} +impl TokioRustlsRuntime { + /// Create a new [`TokioRustlsRuntime`]. + /// + /// The return value will own the underlying Tokio runtime object, which + /// will be dropped when the last copy of this handle is freed. + /// + /// If you want to use a currently running runtime instead, call + /// [`TokioRustlsRuntime::current()`]. + pub fn create() -> IoResult { + crate::impls::tokio::create_runtime().map(|r| TokioRustlsRuntime { + inner: CompoundRuntime::new(r.clone(), r.clone(), r, RustlsProvider::default()), + }) + } -/// Create a new Tokio-based [`Runtime`]. -/// -/// Generally you should call this function only once, and then use -/// [`Clone::clone()`] to create additional references to that -/// runtime. -/// -/// Tokio users may want to avoid this function and instead make a -/// runtime using [`current_runtime()`]: this function always _builds_ a -/// runtime, and if you already have a runtime, that isn't what you -/// want with Tokio. -pub fn create_runtime() -> std::io::Result { - create_tokio_runtime() -} - -/// Create a new Tokio-based [`Runtime`] with `rustls`. -/// -/// Generally you should call this function only once, and then use -/// [`Clone::clone()`] to create additional references to that -/// runtime. -/// -/// Tokio users may want to avoid this function and instead make a -/// runtime using [`current_runtime()`]: this function always _builds_ a -/// runtime, and if you already have a runtime, that isn't what you -/// want with Tokio. -#[cfg(feature = "rustls")] -pub fn create_rustls_runtime() -> std::io::Result { - create_tokio_rustls_runtime() -} - -/// Try to return an instance of the currently running tokio [`Runtime`]. -/// -/// # Usage note -/// -/// We should never call this from inside other Arti crates, or from -/// library crates that want to support multiple runtimes! This -/// function is for Arti _users_ who want to wrap some existing Tokio -/// runtime as a [`Runtime`]. It is not for library -/// crates that want to work with multiple runtimes. -/// -/// Once you have a runtime returned by this function, you should -/// just create more handles to it via [`Clone`]. -pub fn current_runtime() -> std::io::Result { - Ok(current_handle()?.into()) -} - -/// Return an instance of the currently running tokio [`Runtime`], wrapped to -/// use `rustls`. -#[cfg(feature = "rustls")] -pub fn current_runtime_rustls() -> std::io::Result { - Ok(current_handle()?.into()) + /// Return a [`TokioRustlsRuntime`] wrapping the currently running + /// Tokio runtime. + /// + /// # Usage note + /// + /// We should never call this from inside other Arti crates, or from library + /// crates that want to support multiple runtimes! This function is for + /// Arti _users_ who want to wrap some existing Tokio runtime as a + /// [`Runtime`]. It is not for library crates that want to work with + /// multiple runtimes. + /// + /// Once you have a runtime returned by this function, you should just + /// create more handles to it via [`Clone`]. + pub fn current() -> IoResult { + Ok(current_handle()?.into()) + } } /// As `Handle::try_current()`, but return an IoError on failure. @@ -147,6 +145,6 @@ where P: FnOnce(TokioNativeTlsRuntime) -> F, F: futures::Future, { - let runtime = create_tokio_runtime().expect("Failed to create a tokio runtime"); + let runtime = TokioNativeTlsRuntime::create().expect("Failed to create a tokio runtime"); runtime.clone().block_on(func(runtime)) } From 8af3528cd3d90a3aa827a5a9464c686c9250eb78 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 09:52:43 -0500 Subject: [PATCH 08/16] Define aliases for "the best enabled runtime". This helps us simplify our code in a few ways, and will help even more once native_tls is optional. --- crates/tor-rtcompat/src/async_std.rs | 16 +++++++++++--- crates/tor-rtcompat/src/lib.rs | 33 ++++++++++++++-------------- crates/tor-rtcompat/src/test.rs | 4 ++-- crates/tor-rtcompat/src/tokio.rs | 15 +++++++++++-- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/crates/tor-rtcompat/src/async_std.rs b/crates/tor-rtcompat/src/async_std.rs index e672b133f..c8127f319 100644 --- a/crates/tor-rtcompat/src/async_std.rs +++ b/crates/tor-rtcompat/src/async_std.rs @@ -11,6 +11,17 @@ use async_std_crate::net::TcpStream; use async_executors::AsyncStd; +/// An alias for the async_std runtime that we prefer to use, based on whatever TLS +/// implementation has been enabled. +/// +/// If only one of `native_tls` and `rustls` bas been enabled within the +/// `tor-rtcompat` crate, that will be the TLS backend that this uses. +/// +/// Currently, `native_tls` is preferred over `rustls` when both are available, +/// because of its maturity within Arti. However, this might change in the +/// future. +pub use AsyncStdNativeTlsRuntime as PreferredRuntime; + /// A [`Runtime`](crate::Runtime) powered by `async_std` and `native_tls`. #[derive(Clone)] pub struct AsyncStdNativeTlsRuntime { @@ -94,10 +105,9 @@ impl AsyncStdRustlsRuntime { /// Run a test function using a freshly created async_std runtime. pub fn test_with_runtime(func: P) -> O where - P: FnOnce(AsyncStdNativeTlsRuntime) -> F, + P: FnOnce(PreferredRuntime) -> F, F: futures::Future, { - let runtime = - AsyncStdNativeTlsRuntime::create().expect("Couldn't get global async_std runtime?"); + let runtime = PreferredRuntime::create().expect("Couldn't get global async_std runtime?"); runtime.clone().block_on(func(runtime)) } diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index 7ae00362b..06e81ec46 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -171,6 +171,21 @@ pub mod async_std; pub use compound::CompoundRuntime; +#[cfg(all(feature = "async_std", not(feature = "tokio")))] +use async_std as preferred_backend_mod; +#[cfg(feature = "tokio")] +use tokio as preferred_backend_mod; + +/// The runtime that we prefer to use, out of all the runtimes compiled into the +/// tor-rtcompat crate. +/// +/// If `tokio` and `async-std` are both available, we prefer `tokio` for its +/// performance. +/// If `native_tls` and `rustls` are both available, we prefer `native_tls` since +/// it has been used in Arti for longer. +#[cfg(any(feature = "tokio", feature = "async-std"))] +pub use preferred_backend_mod::PreferredRuntime; + /// Try to return an instance of the currently running [`Runtime`]. /// /// # Limitations @@ -190,14 +205,7 @@ pub use compound::CompoundRuntime; /// just create more handles to it via [`Clone`]. #[cfg(any(feature = "async-std", feature = "tokio"))] pub fn current_user_runtime() -> std::io::Result { - #[cfg(feature = "tokio")] - { - crate::tokio::TokioNativeTlsRuntime::current() - } - #[cfg(all(feature = "async-std", not(feature = "tokio")))] - { - crate::async_std::AsyncStdNativeTlsRuntime::current() - } + PreferredRuntime::current() } /// Return a new instance of the default [`Runtime`]. @@ -216,14 +224,7 @@ pub fn current_user_runtime() -> std::io::Result { /// create it using an appropriate builder type or function. #[cfg(any(feature = "async-std", feature = "tokio"))] pub fn create_runtime() -> std::io::Result { - #[cfg(feature = "tokio")] - { - crate::tokio::TokioNativeTlsRuntime::create() - } - #[cfg(all(feature = "async-std", not(feature = "tokio")))] - { - crate::async_std::AsyncStdNativeTlsRuntime::create() - } + PreferredRuntime::create() } /// Helpers for test_with_all_runtimes diff --git a/crates/tor-rtcompat/src/test.rs b/crates/tor-rtcompat/src/test.rs index 40dbe3b77..27ec18859 100644 --- a/crates/tor-rtcompat/src/test.rs +++ b/crates/tor-rtcompat/src/test.rs @@ -225,7 +225,7 @@ macro_rules! runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::tokio::TokioNativeTlsRuntime::create()?) + super::$id(&crate::tokio::PreferredRuntime::create()?) } )* } @@ -235,7 +235,7 @@ macro_rules! runtime_tests { $( #[test] fn $id() -> IoResult<()> { - super::$id(&crate::async_std::AsyncStdNativeTlsRuntime::create()?) + super::$id(&crate::async_std::PreferredRuntime::create()?) } )* } diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index a2f680e98..bff4abe8e 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -9,6 +9,17 @@ use std::io::{Error as IoError, ErrorKind, Result as IoResult}; use crate::impls::rustls::RustlsProvider; use crate::impls::tokio::net::TcpStream; +/// An alias for the Tokio runtime that we prefer to use, based on whatever TLS +/// implementation has been enabled. +/// +/// If only one of `native_tls` and `rustls` bas been enabled within the +/// `tor-rtcompat` crate, that will be the TLS backend that this uses. +/// +/// Currently, `native_tls` is preferred over `rustls` when both are available, +/// because of its maturity within Arti. However, this might change in the +/// future. +pub use TokioNativeTlsRuntime as PreferredRuntime; + /// A [`Runtime`] built around a Handle to a tokio runtime, and `native_tls`. /// /// # Limitations @@ -142,9 +153,9 @@ fn current_handle() -> std::io::Result { /// Panics if we can't create a tokio runtime. pub fn test_with_runtime(func: P) -> O where - P: FnOnce(TokioNativeTlsRuntime) -> F, + P: FnOnce(PreferredRuntime) -> F, F: futures::Future, { - let runtime = TokioNativeTlsRuntime::create().expect("Failed to create a tokio runtime"); + let runtime = PreferredRuntime::create().expect("Failed to create a tokio runtime"); runtime.clone().block_on(func(runtime)) } From 30b3818a9e31b0ebe96d4a32aa9bceb16610f994 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 10:53:06 -0500 Subject: [PATCH 09/16] Make the native-tls crate optional. This commit puts the native-tls crate behind a feature. The feature is off-by-default in the tor-rtcompat crate, but can be enabled either from arti or arti-client. There is an included script that I used to test that tor-rtcompat could build and run its tests with all subsets of its features. Closes #300 --- crates/arti-bench/Cargo.toml | 2 +- crates/arti-client/Cargo.toml | 6 +- crates/arti-client/src/client.rs | 16 +++--- crates/arti/Cargo.toml | 4 +- crates/arti/src/main.rs | 4 +- crates/tor-chanmgr/Cargo.toml | 2 +- crates/tor-circmgr/Cargo.toml | 2 +- crates/tor-dirclient/Cargo.toml | 2 +- crates/tor-dirmgr/Cargo.toml | 4 +- crates/tor-guardmgr/Cargo.toml | 2 +- crates/tor-proto/Cargo.toml | 4 +- crates/tor-rtcompat/Cargo.toml | 14 ++++- crates/tor-rtcompat/misc/matrix.py | 26 +++++++++ crates/tor-rtcompat/src/async_std.rs | 11 +++- crates/tor-rtcompat/src/impls.rs | 1 + crates/tor-rtcompat/src/impls/native_tls.rs | 1 + crates/tor-rtcompat/src/lib.rs | 61 +++++++++++++++++---- crates/tor-rtcompat/src/task.rs | 6 +- crates/tor-rtcompat/src/test.rs | 5 +- crates/tor-rtcompat/src/tokio.rs | 13 ++++- crates/tor-rtmock/Cargo.toml | 2 +- 21 files changed, 143 insertions(+), 45 deletions(-) create mode 100755 crates/tor-rtcompat/misc/matrix.py diff --git a/crates/arti-bench/Cargo.toml b/crates/arti-bench/Cargo.toml index cfb3241e0..c2e742c5a 100644 --- a/crates/arti-bench/Cargo.toml +++ b/crates/arti-bench/Cargo.toml @@ -21,7 +21,7 @@ serde_json = "1.0.50" tracing = "0.1.18" tracing-subscriber = { version = "0.3.0", features = ["env-filter"] } tokio = { version = "1.4", features = ["full"] } -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features = ["tokio"] } +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features = ["tokio", "native-tls"] } arti-config = { path="../arti-config", version = "0.0.3"} arti-client = { package="arti-client", path = "../arti-client", version = "0.0.3"} tokio-socks = "0.5" diff --git a/crates/arti-client/Cargo.toml b/crates/arti-client/Cargo.toml index c9d9f57b0..d9c5276cb 100644 --- a/crates/arti-client/Cargo.toml +++ b/crates/arti-client/Cargo.toml @@ -11,9 +11,11 @@ categories = [ "network-programming", "cryptography" ] repository="https://gitlab.torproject.org/tpo/core/arti.git/" [features] -default = [ "tokio" ] +default = [ "tokio", "native-tls" ] async-std = [ "tor-rtcompat/async-std" ] tokio = [ "tor-rtcompat/tokio", "tor-proto/tokio" ] +native-tls = [ "tor-rtcompat/native-tls" ] +rustls = [ "tor-rtcompat/rustls" ] static = [ "tor-rtcompat/static", "tor-dirmgr/static" ] # Enable experimental APIs that are not yet officially supported. @@ -42,7 +44,7 @@ serde = { version = "1.0.103", features = ["derive"] } thiserror = "1" [dev-dependencies] -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio"] } +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio", "native-tls" ] } tokio-crate = { package = "tokio", version = "1.4", features = ["rt", "rt-multi-thread", "io-util", "net", "time", "macros" ] } hyper = { version = "0.14", features = ["http1", "client", "runtime"] } pin-project = "1" diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index e1e634b2f..84e5c74c3 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -24,9 +24,9 @@ use std::time::Duration; use crate::{status, Error, Result}; #[cfg(feature = "async-std")] -use tor_rtcompat::async_std::AsyncStdNativeTlsRuntime; +use tor_rtcompat::async_std::PreferredRuntime as PreferredAsyncStdRuntime; #[cfg(feature = "tokio")] -use tor_rtcompat::tokio::TokioNativeTlsRuntime; +use tor_rtcompat::tokio::PreferredRuntime as PreferredTokioRuntime; use tracing::{debug, error, info, warn}; /// An active client session on the Tor network. @@ -246,7 +246,7 @@ impl StreamPrefs { } #[cfg(feature = "tokio")] -impl TorClient { +impl TorClient { /// Bootstrap a connection to the Tor network, using the current Tokio runtime. /// /// Returns a client once there is enough directory material to @@ -259,14 +259,14 @@ impl TorClient { /// Panics if called outside of the context of a Tokio runtime. pub async fn bootstrap_with_tokio( config: TorClientConfig, - ) -> Result> { - let rt = TokioNativeTlsRuntime::current().expect("called outside of Tokio runtime"); + ) -> Result> { + let rt = PreferredTokioRuntime::current().expect("called outside of Tokio runtime"); Self::bootstrap(rt, config).await } } #[cfg(feature = "async-std")] -impl TorClient { +impl TorClient { /// Bootstrap a connection to the Tor network, using the current async-std runtime. /// /// Returns a client once there is enough directory material to @@ -275,9 +275,9 @@ impl TorClient { /// This is a convenience wrapper around [`TorClient::bootstrap`]. pub async fn bootstrap_with_async_std( config: TorClientConfig, - ) -> Result> { + ) -> Result> { // FIXME(eta): not actually possible for this to fail - let rt = AsyncStdNativeTlsRuntime::current().expect("failed to get async-std runtime"); + let rt = PreferredAsyncStdRuntime::current().expect("failed to get async-std runtime"); Self::bootstrap(rt, config).await } } diff --git a/crates/arti/Cargo.toml b/crates/arti/Cargo.toml index 790141bb2..363ba76d3 100644 --- a/crates/arti/Cargo.toml +++ b/crates/arti/Cargo.toml @@ -11,9 +11,11 @@ categories = [ "command-line-utilities", "cryptography" ] repository="https://gitlab.torproject.org/tpo/core/arti.git/" [features] -default = [ "tokio" ] +default = [ "tokio", "native-tls" ] async-std = [ "arti-client/async-std", "tor-rtcompat/async-std", "async-ctrlc", "once_cell" ] tokio = [ "tokio-crate", "arti-client/tokio", "tor-rtcompat/tokio" ] +native-tls = [ "arti-client/native-tls", "tor-rtcompat/native-tls" ] +rustls = [ "arti-client/rustls", "tor-rtcompat/rustls" ] static = [ "arti-client/static" ] journald = [ "tracing-journald" ] diff --git a/crates/arti/src/main.rs b/crates/arti/src/main.rs index 0a5a33ce2..c6d36eee9 100644 --- a/crates/arti/src/main.rs +++ b/crates/arti/src/main.rs @@ -230,9 +230,9 @@ fn main() -> Result<()> { process::use_max_file_limit(); #[cfg(all(feature = "async-std", not(feature = "tokio")))] - use tor_rtcompat::tokio::AsyncStdNativeTlsRuntime as ChosenRuntime; + use tor_rtcompat::tokio::PreferredRuntime as ChosenRuntime; #[cfg(feature = "tokio")] - use tor_rtcompat::tokio::TokioNativeTlsRuntime as ChosenRuntime; + use tor_rtcompat::tokio::PreferredRuntime as ChosenRuntime; let runtime = ChosenRuntime::create()?; diff --git a/crates/tor-chanmgr/Cargo.toml b/crates/tor-chanmgr/Cargo.toml index 5bc4a3189..e16221412 100644 --- a/crates/tor-chanmgr/Cargo.toml +++ b/crates/tor-chanmgr/Cargo.toml @@ -30,4 +30,4 @@ float_eq = "0.7" futures-await-test = "0.3.0" hex-literal = "0.3" tor-rtmock = { path="../tor-rtmock", version = "0.0.3"} -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio"] } +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio", "native-tls"] } diff --git a/crates/tor-circmgr/Cargo.toml b/crates/tor-circmgr/Cargo.toml index 3783d0a90..3f16aa018 100644 --- a/crates/tor-circmgr/Cargo.toml +++ b/crates/tor-circmgr/Cargo.toml @@ -50,4 +50,4 @@ tor-guardmgr = { path="../tor-guardmgr", version = "0.0.3", features=["testing"] tor-llcrypto = { path="../tor-llcrypto", version = "0.0.3"} tor-netdir = { path="../tor-netdir", version = "0.0.3", features=["testing"] } tor-persist = { path="../tor-persist", version = "0.0.3", features=["testing"] } -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio"] } +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio", "native-tls" ] } diff --git a/crates/tor-dirclient/Cargo.toml b/crates/tor-dirclient/Cargo.toml index 264d86ded..f4fb49f54 100644 --- a/crates/tor-dirclient/Cargo.toml +++ b/crates/tor-dirclient/Cargo.toml @@ -38,5 +38,5 @@ thiserror = "1" [dev-dependencies] futures-await-test = "0.3.0" tor-rtmock = { path="../tor-rtmock", version = "0.0.3"} -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio"] } +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio", "native-tls" ] } diff --git a/crates/tor-dirmgr/Cargo.toml b/crates/tor-dirmgr/Cargo.toml index 7eba19e5a..ac6903fcd 100644 --- a/crates/tor-dirmgr/Cargo.toml +++ b/crates/tor-dirmgr/Cargo.toml @@ -56,7 +56,5 @@ humantime-serde = "1" futures-await-test = "0.3.0" hex-literal = "0.3" tempfile = "3" -tor-rtcompat = { path = "../tor-rtcompat", version = "0.0.3", features = [ - "tokio", -] } +tor-rtcompat = { path = "../tor-rtcompat", version = "0.0.3", features = [ "tokio", "native-tls" ] } float_eq = "0.7" diff --git a/crates/tor-guardmgr/Cargo.toml b/crates/tor-guardmgr/Cargo.toml index 66e6dfc0f..d6d2521ee 100644 --- a/crates/tor-guardmgr/Cargo.toml +++ b/crates/tor-guardmgr/Cargo.toml @@ -42,5 +42,5 @@ tracing = "0.1.18" tor-netdir = { path="../tor-netdir", version = "0.0.3", features=["testing"]} tor-netdoc = { path="../tor-netdoc", version = "0.0.3"} tor-persist = { path="../tor-persist", version = "0.0.3", features=["testing"]} -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio"]} +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio", "native-tls"]} tor-rtmock = { path="../tor-rtmock", version = "0.0.3"} diff --git a/crates/tor-proto/Cargo.toml b/crates/tor-proto/Cargo.toml index 3e814b48e..6c07f5674 100644 --- a/crates/tor-proto/Cargo.toml +++ b/crates/tor-proto/Cargo.toml @@ -49,8 +49,6 @@ tokio-util = { version = "0.6", features = ["compat"], optional = true } coarsetime = { version = "0.1.20", optional = true } [dev-dependencies] -tor-rtcompat = { path = "../tor-rtcompat", version = "0.0.3", features = [ - "tokio", -] } +tor-rtcompat = { path = "../tor-rtcompat", version = "0.0.3", features = [ "tokio", "native-tls" ] } hex-literal = "0.3" hex = "0.4" diff --git a/crates/tor-rtcompat/Cargo.toml b/crates/tor-rtcompat/Cargo.toml index 3770ca925..133f55ebd 100644 --- a/crates/tor-rtcompat/Cargo.toml +++ b/crates/tor-rtcompat/Cargo.toml @@ -15,7 +15,10 @@ repository="https://gitlab.torproject.org/tpo/core/arti.git/" default = [ ] async-std = [ "async-std-crate", "async-io", "async_executors/async_std" ] tokio = [ "tokio-crate", "tokio-util", "async_executors/tokio_tp" ] -static = [ "native-tls/vendored" ] +# TODO: This feature makes us link native-tls statically even if we +# don't want to use native-tls in the first place. That's not so clever! +static = [ "native-tls-crate/vendored" ] +native-tls = [ "native-tls-crate", "async-native-tls" ] rustls = [ "rustls-crate", "async-rustls", "x509-signature" ] [dependencies] @@ -24,15 +27,20 @@ async_executors = { version = "0.4", default_features = false } async-trait = "0.1.2" futures = "0.3" pin-project = "1" -native-tls = "0.2" +native-tls-crate = { package = "native-tls", version = "0.2", optional = true } rustls-crate = { package = "rustls", version = "0.19", optional = true, features = [ "dangerous_configuration" ] } async-std-crate = { package = "async-std", version = "1.7.0", optional = true } async-io = { version = "1.4.1", optional = true } -async-native-tls = { version = "0.4.0" } +async-native-tls = { version = "0.4.0", optional = true } tokio-crate = { package = "tokio", version = "1.4", optional = true, features = ["rt", "rt-multi-thread", "io-util", "net", "time" ] } tokio-util = { version = "0.6", features = ["compat"], optional = true } async-rustls = { version = "0.2.0", optional = true } x509-signature = { version = "0.5.0", optional = true } + +[dev-dependencies] +# Used for testing our TLS implementation. +native-tls-crate = { package = "native-tls", version = "0.2" } + diff --git a/crates/tor-rtcompat/misc/matrix.py b/crates/tor-rtcompat/misc/matrix.py new file mode 100755 index 000000000..4031a2490 --- /dev/null +++ b/crates/tor-rtcompat/misc/matrix.py @@ -0,0 +1,26 @@ +#!/usr/bin/python +# +# Run a provided command over all possible subsets of important +# tor-rtcompat features. + +import sys, subprocess + +if sys.argv[1:] == []: + print("You need to name a program to run with different features") + sys.exit(1) + +FEATURES = [ "tokio", "async-std", "native-tls", "rustls" ] + +COMBINATIONS = [ [] ] + +# Generate all combinations of features. +for feature in FEATURES: + new_combinations = [ c + [ feature ] for c in COMBINATIONS ] + COMBINATIONS.extend(new_combinations) + +for c in COMBINATIONS: + arg = "--features={}".format(",".join(c)) + commandline = sys.argv[1:] + [arg] + print(" ".join(commandline)) + subprocess.check_call(commandline) + diff --git a/crates/tor-rtcompat/src/async_std.rs b/crates/tor-rtcompat/src/async_std.rs index c8127f319..23c00a3ea 100644 --- a/crates/tor-rtcompat/src/async_std.rs +++ b/crates/tor-rtcompat/src/async_std.rs @@ -3,8 +3,8 @@ pub use crate::impls::async_std::create_runtime as create_runtime_impl; use crate::{compound::CompoundRuntime, SpawnBlocking}; use std::io::Result as IoResult; +#[cfg(feature = "native-tls")] use crate::impls::native_tls::NativeTlsProvider; - #[cfg(feature = "rustls")] use crate::impls::rustls::RustlsProvider; use async_std_crate::net::TcpStream; @@ -20,18 +20,25 @@ use async_executors::AsyncStd; /// Currently, `native_tls` is preferred over `rustls` when both are available, /// because of its maturity within Arti. However, this might change in the /// future. +#[cfg(all(feature = "native-tls"))] pub use AsyncStdNativeTlsRuntime as PreferredRuntime; +#[cfg(all(feature = "rustls", not(feature = "native-tls")))] +pub use AsyncStdRustlsRuntime as PreferredRuntime; + /// A [`Runtime`](crate::Runtime) powered by `async_std` and `native_tls`. #[derive(Clone)] +#[cfg(all(feature = "native-tls"))] pub struct AsyncStdNativeTlsRuntime { /// The actual runtime object. inner: NativeTlsInner, } /// Implementation type for AsyncStdRuntime. +#[cfg(all(feature = "native-tls"))] type NativeTlsInner = CompoundRuntime>; +#[cfg(all(feature = "native-tls"))] crate::opaque::implement_opaque_runtime! { AsyncStdNativeTlsRuntime { inner : NativeTlsInner } } @@ -53,6 +60,7 @@ crate::opaque::implement_opaque_runtime! { AsyncStdRustlsRuntime { inner: RustlsInner } } +#[cfg(all(feature = "native-tls"))] impl AsyncStdNativeTlsRuntime { /// Return a new [`AsyncStdNativeTlsRuntime`] /// @@ -103,6 +111,7 @@ impl AsyncStdRustlsRuntime { } /// Run a test function using a freshly created async_std runtime. +#[cfg(any(feature = "native-tls", feature = "rustls"))] pub fn test_with_runtime(func: P) -> O where P: FnOnce(PreferredRuntime) -> F, diff --git a/crates/tor-rtcompat/src/impls.rs b/crates/tor-rtcompat/src/impls.rs index 9efcde0cf..6ef05085e 100644 --- a/crates/tor-rtcompat/src/impls.rs +++ b/crates/tor-rtcompat/src/impls.rs @@ -11,4 +11,5 @@ pub(crate) mod tokio; #[cfg(all(feature = "rustls"))] pub(crate) mod rustls; +#[cfg(all(feature = "native-tls"))] pub(crate) mod native_tls; diff --git a/crates/tor-rtcompat/src/impls/native_tls.rs b/crates/tor-rtcompat/src/impls/native_tls.rs index a0de354da..e1a7f72ab 100644 --- a/crates/tor-rtcompat/src/impls/native_tls.rs +++ b/crates/tor-rtcompat/src/impls/native_tls.rs @@ -4,6 +4,7 @@ use crate::traits::{CertifiedConn, TlsConnector, TlsProvider}; use async_trait::async_trait; use futures::{AsyncRead, AsyncWrite}; +use native_tls_crate as native_tls; use std::{ convert::TryInto, io::{Error as IoError, Result as IoResult}, diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index 06e81ec46..30542d1ef 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -140,6 +140,10 @@ #![warn(clippy::unseparated_literal_suffix)] #![deny(clippy::unwrap_used)] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + any(feature = "async-std", feature = "tokio") +))] pub(crate) mod impls; pub mod task; @@ -148,7 +152,11 @@ mod opaque; mod timer; mod traits; -#[cfg(all(test, any(feature = "tokio", feature = "async-std")))] +#[cfg(all( + test, + any(feature = "native-tls", feature = "rustls"), + any(feature = "async-std", feature = "tokio") +))] mod test; pub use traits::{ @@ -163,17 +171,21 @@ pub mod tls { pub use crate::traits::{CertifiedConn, TlsConnector}; } -#[cfg(feature = "tokio")] +#[cfg(all(any(feature = "native-tls", feature = "rustls"), feature = "tokio"))] pub mod tokio; -#[cfg(feature = "async-std")] +#[cfg(all(any(feature = "native-tls", feature = "rustls"), feature = "async-std"))] pub mod async_std; pub use compound::CompoundRuntime; -#[cfg(all(feature = "async_std", not(feature = "tokio")))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + feature = "async-std", + not(feature = "tokio") +))] use async_std as preferred_backend_mod; -#[cfg(feature = "tokio")] +#[cfg(all(any(feature = "native-tls", feature = "rustls"), feature = "tokio"))] use tokio as preferred_backend_mod; /// The runtime that we prefer to use, out of all the runtimes compiled into the @@ -183,7 +195,10 @@ use tokio as preferred_backend_mod; /// performance. /// If `native_tls` and `rustls` are both available, we prefer `native_tls` since /// it has been used in Arti for longer. -#[cfg(any(feature = "tokio", feature = "async-std"))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + any(feature = "async-std", feature = "tokio") +))] pub use preferred_backend_mod::PreferredRuntime; /// Try to return an instance of the currently running [`Runtime`]. @@ -203,7 +218,10 @@ pub use preferred_backend_mod::PreferredRuntime; /// /// Once you have a runtime returned by this function, you should /// just create more handles to it via [`Clone`]. -#[cfg(any(feature = "async-std", feature = "tokio"))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + any(feature = "async-std", feature = "tokio") +))] pub fn current_user_runtime() -> std::io::Result { PreferredRuntime::current() } @@ -222,7 +240,10 @@ pub fn current_user_runtime() -> std::io::Result { /// /// If you need more fine-grained control over a runtime, you can /// create it using an appropriate builder type or function. -#[cfg(any(feature = "async-std", feature = "tokio"))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + any(feature = "async-std", feature = "tokio") +))] pub fn create_runtime() -> std::io::Result { PreferredRuntime::create() } @@ -249,7 +270,11 @@ pub mod testing__ { /// (This is a macro so that it can repeat the closure as two separate /// expressions, so it can take on two different types, if needed.) #[macro_export] -#[cfg(all(feature = "tokio", feature = "async-std"))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + feature = "tokio", + feature = "async-std" +))] macro_rules! test_with_all_runtimes { ( $fn:expr ) => {{ use $crate::testing__::TestOutcome; @@ -260,7 +285,11 @@ macro_rules! test_with_all_runtimes { /// Run a test closure, passing as argument every supported runtime. #[macro_export] -#[cfg(all(feature = "tokio", not(feature = "async-std")))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + feature = "tokio", + not(feature = "async-std") +))] macro_rules! test_with_all_runtimes { ( $fn:expr ) => {{ $crate::tokio::test_with_runtime($fn) @@ -269,7 +298,11 @@ macro_rules! test_with_all_runtimes { /// Run a test closure, passing as argument every supported runtime. #[macro_export] -#[cfg(all(not(feature = "tokio"), feature = "async-std"))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + not(feature = "tokio"), + feature = "async-std" +))] macro_rules! test_with_all_runtimes { ( $fn:expr ) => {{ $crate::async_std::test_with_runtime($fn) @@ -291,7 +324,11 @@ macro_rules! test_with_one_runtime { /// /// (Always prefers tokio if present.) #[macro_export] -#[cfg(all(not(feature = "tokio"), feature = "async-std"))] +#[cfg(all( + any(feature = "native-tls", feature = "rustls"), + not(feature = "tokio"), + feature = "async-std" +))] macro_rules! test_with_one_runtime { ( $fn:expr ) => {{ $crate::async_std::test_with_runtime($fn) diff --git a/crates/tor-rtcompat/src/task.rs b/crates/tor-rtcompat/src/task.rs index 84b97c8f5..5ee7eaa00 100644 --- a/crates/tor-rtcompat/src/task.rs +++ b/crates/tor-rtcompat/src/task.rs @@ -38,7 +38,11 @@ impl Future for YieldFuture { } } -#[cfg(all(test, any(feature = "tokio", feature = "async-std")))] +#[cfg(all( + test, + any(feature = "native-tls", feature = "rustls"), + any(feature = "tokio", feature = "async-std") +))] mod test { use super::yield_now; use crate::test_with_all_runtimes; diff --git a/crates/tor-rtcompat/src/test.rs b/crates/tor-rtcompat/src/test.rs index 27ec18859..6d3c4e5e2 100644 --- a/crates/tor-rtcompat/src/test.rs +++ b/crates/tor-rtcompat/src/test.rs @@ -6,6 +6,7 @@ use crate::traits::*; use futures::io::{AsyncReadExt, AsyncWriteExt}; use futures::stream::StreamExt; +use native_tls_crate as native_tls; use std::io::Result as IoResult; use std::net::{Ipv4Addr, SocketAddrV4}; use std::time::{Duration, Instant, SystemTime}; @@ -244,7 +245,7 @@ macro_rules! runtime_tests { macro_rules! tls_runtime_tests { { $($id:ident),* $(,)? } => { - #[cfg(feature="tokio")] + #[cfg(all(feature="tokio", feature = "native-tls"))] mod tokio_native_tls_tests { use std::io::Result as IoResult; $( @@ -254,7 +255,7 @@ macro_rules! tls_runtime_tests { } )* } - #[cfg(feature="async-std")] + #[cfg(all(feature="async-std", feature = "native-tls"))] mod async_std_native_tls_tests { use std::io::Result as IoResult; $( diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index bff4abe8e..c7f890dfb 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -1,10 +1,11 @@ //! Entry points for use with Tokio runtimes. -use crate::impls::native_tls::NativeTlsProvider; use crate::impls::tokio::TokioRuntimeHandle as Handle; use crate::{CompoundRuntime, SpawnBlocking}; use std::io::{Error as IoError, ErrorKind, Result as IoResult}; +#[cfg(feature = "native-tls")] +use crate::impls::native_tls::NativeTlsProvider; #[cfg(feature = "rustls")] use crate::impls::rustls::RustlsProvider; use crate::impls::tokio::net::TcpStream; @@ -18,7 +19,10 @@ use crate::impls::tokio::net::TcpStream; /// Currently, `native_tls` is preferred over `rustls` when both are available, /// because of its maturity within Arti. However, this might change in the /// future. +#[cfg(feature = "native-tls")] pub use TokioNativeTlsRuntime as PreferredRuntime; +#[cfg(all(feature = "rustls", not(feature = "native-tls")))] +pub use TokioRustlsRuntime as PreferredRuntime; /// A [`Runtime`] built around a Handle to a tokio runtime, and `native_tls`. /// @@ -28,12 +32,14 @@ pub use TokioNativeTlsRuntime as PreferredRuntime; /// implementations for Tokio's time, net, and io facilities, but we have /// no good way to check that when creating this object. #[derive(Clone)] +#[cfg(feature = "native-tls")] pub struct TokioNativeTlsRuntime { /// The actual [`CompoundRuntime`] that implements this. inner: HandleInner, } /// Implementation type for a TokioRuntimeHandle. +#[cfg(feature = "native-tls")] type HandleInner = CompoundRuntime>; /// A [`Runtime`] built around a Handle to a tokio runtime, and `rustls`. @@ -48,6 +54,7 @@ pub struct TokioRustlsRuntime { #[cfg(feature = "rustls")] type RustlsHandleInner = CompoundRuntime>; +#[cfg(feature = "native-tls")] crate::opaque::implement_opaque_runtime! { TokioNativeTlsRuntime { inner : HandleInner } } @@ -57,6 +64,7 @@ crate::opaque::implement_opaque_runtime! { TokioRustlsRuntime { inner : RustlsHandleInner } } +#[cfg(feature = "native-tls")] impl From for TokioNativeTlsRuntime { fn from(h: tokio_crate::runtime::Handle) -> Self { let h = Handle::new(h); @@ -76,6 +84,7 @@ impl From for TokioRustlsRuntime { } } +#[cfg(feature = "native-tls")] impl TokioNativeTlsRuntime { /// Create a new [`TokioNativeTlsRuntime`]. /// @@ -142,6 +151,7 @@ impl TokioRustlsRuntime { } /// As `Handle::try_current()`, but return an IoError on failure. +#[cfg(any(feature = "native-tls", feature = "rustls"))] fn current_handle() -> std::io::Result { tokio_crate::runtime::Handle::try_current().map_err(|e| IoError::new(ErrorKind::Other, e)) } @@ -151,6 +161,7 @@ fn current_handle() -> std::io::Result { /// # Panics /// /// Panics if we can't create a tokio runtime. +#[cfg(any(feature = "native-tls", feature = "rustls"))] pub fn test_with_runtime(func: P) -> O where P: FnOnce(PreferredRuntime) -> F, diff --git a/crates/tor-rtmock/Cargo.toml b/crates/tor-rtmock/Cargo.toml index 4299fd96d..ab7b6e6b0 100644 --- a/crates/tor-rtmock/Cargo.toml +++ b/crates/tor-rtmock/Cargo.toml @@ -22,4 +22,4 @@ tor-rtcompat = { version = "0.0.3", path = "../tor-rtcompat" } [dev-dependencies] futures-await-test = "0.3.0" rand = "0.8" -tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio"] } +tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", features=["tokio", "native-tls" ] } From dec2c4ee639c3a3584f416116f286231d0a59df8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 13:27:42 -0500 Subject: [PATCH 10/16] Make test_with_all_runtimes cover _all_ the runtimes. This took some refactoring, so that I wouldn't need to define 9 different versions of the function. It also required that we change the behavior of test_with_all_runtimes slightly, so that it asserts on _any_ failure rather than asserting on most but returning Err() for others. That in turn required changes to a few of its callers. There's probably a better way to do all of this macro business, but this is the best I could find. --- crates/tor-rtcompat/src/async_std.rs | 37 ++++++--- crates/tor-rtcompat/src/lib.rs | 117 ++++++++++++++++----------- crates/tor-rtcompat/src/task.rs | 5 +- crates/tor-rtcompat/src/tokio.rs | 43 ++++++---- crates/tor-rtmock/src/net.rs | 18 +++-- crates/tor-rtmock/src/time.rs | 4 +- 6 files changed, 139 insertions(+), 85 deletions(-) diff --git a/crates/tor-rtcompat/src/async_std.rs b/crates/tor-rtcompat/src/async_std.rs index 23c00a3ea..8778d6dff 100644 --- a/crates/tor-rtcompat/src/async_std.rs +++ b/crates/tor-rtcompat/src/async_std.rs @@ -83,6 +83,20 @@ impl AsyncStdNativeTlsRuntime { pub fn current() -> IoResult { Self::create() } + + /// Helper to run a single test function in a freshly created runtime. + /// + /// # Panics + /// + /// Panics if we can't create this runtime. + pub fn run_test(func: P) -> O + where + P: FnOnce(Self) -> F, + F: futures::Future, + { + let runtime = Self::create().expect("Failed to create runtime"); + runtime.clone().block_on(func(runtime)) + } } #[cfg(feature = "rustls")] @@ -108,15 +122,18 @@ impl AsyncStdRustlsRuntime { pub fn current() -> IoResult { Self::create() } -} -/// Run a test function using a freshly created async_std runtime. -#[cfg(any(feature = "native-tls", feature = "rustls"))] -pub fn test_with_runtime(func: P) -> O -where - P: FnOnce(PreferredRuntime) -> F, - F: futures::Future, -{ - let runtime = PreferredRuntime::create().expect("Couldn't get global async_std runtime?"); - runtime.clone().block_on(func(runtime)) + /// Helper to run a single test function in a freshly created runtime. + /// + /// # Panics + /// + /// Panics if we can't create this runtime. + pub fn run_test(func: P) -> O + where + P: FnOnce(Self) -> F, + F: futures::Future, + { + let runtime = Self::create().expect("Failed to create runtime"); + runtime.clone().block_on(func(runtime)) + } } diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index 30542d1ef..022d185bf 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -250,7 +250,8 @@ pub fn create_runtime() -> std::io::Result { /// Helpers for test_with_all_runtimes pub mod testing__ { - /// A trait for an object that might represent a test failure. + /// A trait for an object that might represent a test failure, or which + /// might just be `(). pub trait TestOutcome { /// Abort if the test has failed. fn check_ok(&self); @@ -258,65 +259,88 @@ pub mod testing__ { impl TestOutcome for () { fn check_ok(&self) {} } - impl TestOutcome for Result { + impl TestOutcome for Result<(), E> { fn check_ok(&self) { - assert!(self.is_ok()); + self.as_ref().expect("Test failure"); } } } +/// Helper: define a macro that expands a token tree iff a pair of features are +/// both present. +macro_rules! declare_conditional_macro { + ( $(#[$meta:meta])* macro $name:ident = ($f1:expr, $f2:expr) ) => { + $( #[$meta] )* + #[cfg(all(feature=$f1, feature=$f2))] + #[macro_export] + macro_rules! $name { + ($tt:tt) => { + $tt + }; + } + + $( #[$meta] )* + #[cfg(not(all(feature=$f1, feature=$f2)))] + #[macro_export] + macro_rules! $name { + ($tt:tt) => {}; + } + + // Needed so that we can access this macro at this path, both within the + // crate and without. + pub use $name; + }; +} + +/// Defines macros that will expand when certain runtimes are available. +pub mod cond { + declare_conditional_macro! { + /// Expand a token tree if the TokioNativeTlsRuntime is available. + macro if_tokio_native_tls_present = ("tokio", "native-tls") + } + declare_conditional_macro! { + /// Expand a token tree if the TokioRustlsRuntime is available. + macro if_tokio_rustls_present = ("tokio", "rustls") + } + declare_conditional_macro! { + /// Expand a token tree if the TokioNativeTlsRuntime is available. + macro if_async_std_native_tls_present = ("async-std", "native-tls") + } + declare_conditional_macro! { + /// Expand a token tree if the TokioNativeTlsRuntime is available. + macro if_async_std_rustls_present = ("async-std", "rustls") + } +} + /// Run a test closure, passing as argument every supported runtime. /// -/// (This is a macro so that it can repeat the closure as two separate +/// (This is a macro so that it can repeat the closure as multiple separate /// expressions, so it can take on two different types, if needed.) #[macro_export] #[cfg(all( any(feature = "native-tls", feature = "rustls"), - feature = "tokio", - feature = "async-std" + any(feature = "tokio", feature = "async-std"), ))] macro_rules! test_with_all_runtimes { ( $fn:expr ) => {{ + use $crate::cond::*; use $crate::testing__::TestOutcome; - $crate::tokio::test_with_runtime($fn).check_ok(); - $crate::async_std::test_with_runtime($fn) - }}; -} + // We have to do this outcome-checking business rather than just using + // the ? operator or calling expect() because some of the closures that + // we use this macro with return (), and some return Result. -/// Run a test closure, passing as argument every supported runtime. -#[macro_export] -#[cfg(all( - any(feature = "native-tls", feature = "rustls"), - feature = "tokio", - not(feature = "async-std") -))] -macro_rules! test_with_all_runtimes { - ( $fn:expr ) => {{ - $crate::tokio::test_with_runtime($fn) - }}; -} - -/// Run a test closure, passing as argument every supported runtime. -#[macro_export] -#[cfg(all( - any(feature = "native-tls", feature = "rustls"), - not(feature = "tokio"), - feature = "async-std" -))] -macro_rules! test_with_all_runtimes { - ( $fn:expr ) => {{ - $crate::async_std::test_with_runtime($fn) - }}; -} - -/// Run a test closure, passing as argument one supported runtime. -/// -/// (Always prefers tokio if present.) -#[macro_export] -#[cfg(feature = "tokio")] -macro_rules! test_with_one_runtime { - ( $fn:expr ) => {{ - $crate::tokio::test_with_runtime($fn) + if_tokio_native_tls_present! {{ + $crate::tokio::TokioNativeTlsRuntime::run_test($fn).check_ok(); + }} + if_tokio_rustls_present! {{ + $crate::tokio::TokioRustlsRuntime::run_test($fn).check_ok(); + }} + if_async_std_native_tls_present! {{ + $crate::async_std::AsyncStdNativeTlsRuntime::run_test($fn).check_ok(); + }} + if_async_std_rustls_present! {{ + $crate::async_std::AsyncStdRustlsRuntime::run_test($fn).check_ok(); + }} }}; } @@ -326,11 +350,10 @@ macro_rules! test_with_one_runtime { #[macro_export] #[cfg(all( any(feature = "native-tls", feature = "rustls"), - not(feature = "tokio"), - feature = "async-std" + any(feature = "tokio", feature = "async-std"), ))] macro_rules! test_with_one_runtime { ( $fn:expr ) => {{ - $crate::async_std::test_with_runtime($fn) + $crate::PreferredRuntime::run_test($fn) }}; } diff --git a/crates/tor-rtcompat/src/task.rs b/crates/tor-rtcompat/src/task.rs index 5ee7eaa00..7e792a2d9 100644 --- a/crates/tor-rtcompat/src/task.rs +++ b/crates/tor-rtcompat/src/task.rs @@ -50,7 +50,7 @@ mod test { use std::sync::atomic::{AtomicBool, Ordering}; #[test] - fn test_yield() -> std::io::Result<()> { + fn test_yield() { test_with_all_runtimes!(|_| async { let b = AtomicBool::new(false); use Ordering::SeqCst; @@ -81,7 +81,6 @@ mod test { } ); std::io::Result::Ok(()) - })?; - Ok(()) + }); } } diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index c7f890dfb..d5589dfed 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -115,6 +115,20 @@ impl TokioNativeTlsRuntime { pub fn current() -> IoResult { Ok(current_handle()?.into()) } + + /// Helper to run a single test function in a freshly created runtime. + /// + /// # Panics + /// + /// Panics if we can't create this runtime. + pub fn run_test(func: P) -> O + where + P: FnOnce(Self) -> F, + F: futures::Future, + { + let runtime = Self::create().expect("Failed to create runtime"); + runtime.clone().block_on(func(runtime)) + } } #[cfg(feature = "rustls")] @@ -148,6 +162,20 @@ impl TokioRustlsRuntime { pub fn current() -> IoResult { Ok(current_handle()?.into()) } + + /// Helper to run a single test function in a freshly created runtime. + /// + /// # Panics + /// + /// Panics if we can't create this runtime. + pub fn run_test(func: P) -> O + where + P: FnOnce(Self) -> F, + F: futures::Future, + { + let runtime = Self::create().expect("Failed to create runtime"); + runtime.clone().block_on(func(runtime)) + } } /// As `Handle::try_current()`, but return an IoError on failure. @@ -155,18 +183,3 @@ impl TokioRustlsRuntime { fn current_handle() -> std::io::Result { tokio_crate::runtime::Handle::try_current().map_err(|e| IoError::new(ErrorKind::Other, e)) } - -/// Run a test function using a freshly created tokio runtime. -/// -/// # Panics -/// -/// Panics if we can't create a tokio runtime. -#[cfg(any(feature = "native-tls", feature = "rustls"))] -pub fn test_with_runtime(func: P) -> O -where - P: FnOnce(PreferredRuntime) -> F, - F: futures::Future, -{ - let runtime = PreferredRuntime::create().expect("Failed to create a tokio runtime"); - runtime.clone().block_on(func(runtime)) -} diff --git a/crates/tor-rtmock/src/net.rs b/crates/tor-rtmock/src/net.rs index c142f5c12..983948d46 100644 --- a/crates/tor-rtmock/src/net.rs +++ b/crates/tor-rtmock/src/net.rs @@ -502,7 +502,7 @@ mod test { } #[test] - fn end_to_end() -> IoResult<()> { + fn end_to_end() { test_with_all_runtimes!(|_rt| async { let (client1, client2) = client_pair(); let lis = client2.listen(&"0.0.0.0:99".parse().unwrap()).await?; @@ -532,7 +532,7 @@ mod test { r1?; r2?; IoResult::Ok(()) - }) + }); } #[test] @@ -573,7 +573,7 @@ mod test { } #[test] - fn listener_stream() -> IoResult<()> { + fn listener_stream() { test_with_all_runtimes!(|_rt| async { let (client1, client2) = client_pair(); @@ -602,16 +602,18 @@ mod test { r1?; r2?; IoResult::Ok(()) - }) + }); } #[test] - fn tls_basics() -> IoResult<()> { + fn tls_basics() { let (client1, client2) = client_pair(); let cert = b"I am certified for something I assure you."; - let lis = client2.listen_tls(&"0.0.0.0:0".parse().unwrap(), cert[..].into())?; - let address = lis.local_addr()?; + let lis = client2 + .listen_tls(&"0.0.0.0:0".parse().unwrap(), cert[..].into()) + .unwrap(); + let address = lis.local_addr().unwrap(); test_with_all_runtimes!(|_rt| async { let (r1, r2): (IoResult<()>, IoResult<()>) = futures::join!( @@ -642,6 +644,6 @@ mod test { r1?; r2?; IoResult::Ok(()) - }) + }); } } diff --git a/crates/tor-rtmock/src/time.rs b/crates/tor-rtmock/src/time.rs index 0509e4343..c4d703e7b 100644 --- a/crates/tor-rtmock/src/time.rs +++ b/crates/tor-rtmock/src/time.rs @@ -455,7 +455,7 @@ mod test { } #[test] - fn time_moves_on() -> std::io::Result<()> { + fn time_moves_on() { test_with_all_runtimes!(|_| async { use futures::channel::oneshot; use std::sync::atomic::AtomicBool; @@ -514,6 +514,6 @@ mod test { } ); std::io::Result::Ok(()) - }) + }); } } From bf8fa66d36298561cc868706f748049cec23f5eb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 13:59:37 -0500 Subject: [PATCH 11/16] Rename `SpawnBlocking` trait to `BlockOn`. This avoids a future confusion with the new `SpawnBlocking` trait in async_executors v0.5, and better describes what the trait provides. --- crates/arti/src/main.rs | 2 +- crates/tor-rtcompat/src/async_std.rs | 2 +- crates/tor-rtcompat/src/compound.rs | 8 ++++---- crates/tor-rtcompat/src/impls/async_std.rs | 2 +- crates/tor-rtcompat/src/impls/tokio.rs | 2 +- crates/tor-rtcompat/src/lib.rs | 6 +++--- crates/tor-rtcompat/src/opaque.rs | 2 +- crates/tor-rtcompat/src/tokio.rs | 2 +- crates/tor-rtcompat/src/traits.rs | 8 ++++---- crates/tor-rtmock/src/net_runtime.rs | 4 ++-- crates/tor-rtmock/src/sleep_runtime.rs | 4 ++-- 11 files changed, 21 insertions(+), 21 deletions(-) diff --git a/crates/arti/src/main.rs b/crates/arti/src/main.rs index c6d36eee9..14693c14b 100644 --- a/crates/arti/src/main.rs +++ b/crates/arti/src/main.rs @@ -96,7 +96,7 @@ use std::sync::Arc; use arti_client::{TorClient, TorClientConfig}; use arti_config::ArtiConfig; -use tor_rtcompat::{Runtime, SpawnBlocking}; +use tor_rtcompat::{BlockOn, Runtime}; use anyhow::Result; use clap::{App, AppSettings, Arg, SubCommand}; diff --git a/crates/tor-rtcompat/src/async_std.rs b/crates/tor-rtcompat/src/async_std.rs index 8778d6dff..14cb16253 100644 --- a/crates/tor-rtcompat/src/async_std.rs +++ b/crates/tor-rtcompat/src/async_std.rs @@ -1,6 +1,6 @@ //! Entry points for use with async_std runtimes. pub use crate::impls::async_std::create_runtime as create_runtime_impl; -use crate::{compound::CompoundRuntime, SpawnBlocking}; +use crate::{compound::CompoundRuntime, BlockOn}; use std::io::Result as IoResult; #[cfg(feature = "native-tls")] diff --git a/crates/tor-rtcompat/src/compound.rs b/crates/tor-rtcompat/src/compound.rs index 8df5157f3..6345ac7fa 100644 --- a/crates/tor-rtcompat/src/compound.rs +++ b/crates/tor-rtcompat/src/compound.rs @@ -10,7 +10,7 @@ use std::io::Result as IoResult; /// A runtime made of several parts, each of which implements one trait-group. /// -/// The `SpawnR` component should implements [`Spawn`] and [`SpawnBlocking`]; +/// The `SpawnR` component should implements [`Spawn`] and [`BlockOn`]; /// the `SleepR` component should implement [`SleepProvider`]; the `TcpR` /// component should implement [`TcpProvider`]; and the `TlsR` component should /// implement [`TlsProvider`]. @@ -38,7 +38,7 @@ impl Clone for CompoundRuntime { - /// A `Spawn` and `SpawnBlocking` implementation. + /// A `Spawn` and `BlockOn` implementation. spawn: SpawnR, /// A `SleepProvider` implementation. sleep: SleepR, @@ -72,9 +72,9 @@ where } } -impl SpawnBlocking for CompoundRuntime +impl BlockOn for CompoundRuntime where - SpawnR: SpawnBlocking, + SpawnR: BlockOn, { #[inline] fn block_on(&self, future: F) -> F::Output { diff --git a/crates/tor-rtcompat/src/impls/async_std.rs b/crates/tor-rtcompat/src/impls/async_std.rs index 08b1748a5..4e6c2d8e7 100644 --- a/crates/tor-rtcompat/src/impls/async_std.rs +++ b/crates/tor-rtcompat/src/impls/async_std.rs @@ -131,7 +131,7 @@ impl SleepProvider for async_executors::AsyncStd { } } -impl SpawnBlocking for async_executors::AsyncStd { +impl BlockOn for async_executors::AsyncStd { fn block_on(&self, f: F) -> F::Output { async_executors::AsyncStd::block_on(f) } diff --git a/crates/tor-rtcompat/src/impls/tokio.rs b/crates/tor-rtcompat/src/impls/tokio.rs index 85f207617..a28b66331 100644 --- a/crates/tor-rtcompat/src/impls/tokio.rs +++ b/crates/tor-rtcompat/src/impls/tokio.rs @@ -215,7 +215,7 @@ impl From for TokioRuntimeHandle { } } -impl SpawnBlocking for TokioRuntimeHandle { +impl BlockOn for TokioRuntimeHandle { fn block_on(&self, f: F) -> F::Output { self.handle.block_on(f) } diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index 022d185bf..efcdd8abd 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -40,7 +40,7 @@ //! The `tor-rtcompat` crate provides several traits that //! encapsulate different runtime capabilities. //! -//! * A runtime is a [`SpawnBlocking`] if it can block on a future. +//! * A runtime is a [`BlockOn`] if it can block on a future. //! * A runtime is a [`SleepProvider`] if it can make timer futures that //! become Ready after a given interval of time. //! * A runtime is a [`TcpProvider`] if it can make and receive TCP @@ -92,7 +92,7 @@ //! We could simplify this code significantly by removing most of the //! traits it exposes, and instead just exposing a single //! implementation. For example, instead of exposing a -//! [`SpawnBlocking`] trait to represent blocking until a task is +//! [`BlockOn`] trait to represent blocking until a task is //! done, we could just provide a single global `block_on` function. //! //! That simplification would come at a cost, however. First of all, @@ -160,7 +160,7 @@ mod traits; mod test; pub use traits::{ - CertifiedConn, Runtime, SleepProvider, SpawnBlocking, TcpListener, TcpProvider, TlsProvider, + BlockOn, CertifiedConn, Runtime, SleepProvider, TcpListener, TcpProvider, TlsProvider, }; pub use timer::{SleepProviderExt, Timeout, TimeoutError}; diff --git a/crates/tor-rtcompat/src/opaque.rs b/crates/tor-rtcompat/src/opaque.rs index 111b73773..65a6ee40f 100644 --- a/crates/tor-rtcompat/src/opaque.rs +++ b/crates/tor-rtcompat/src/opaque.rs @@ -16,7 +16,7 @@ macro_rules! implement_opaque_runtime { } } - impl $crate::traits::SpawnBlocking for $t { + impl $crate::traits::BlockOn for $t { #[inline] fn block_on(&self, future: F) -> F::Output { self.$member.block_on(future) diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index d5589dfed..7a504e56d 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -1,7 +1,7 @@ //! Entry points for use with Tokio runtimes. use crate::impls::tokio::TokioRuntimeHandle as Handle; -use crate::{CompoundRuntime, SpawnBlocking}; +use crate::{BlockOn, CompoundRuntime}; use std::io::{Error as IoError, ErrorKind, Result as IoResult}; #[cfg(feature = "native-tls")] diff --git a/crates/tor-rtcompat/src/traits.rs b/crates/tor-rtcompat/src/traits.rs index 443e0a436..c6a9d9172 100644 --- a/crates/tor-rtcompat/src/traits.rs +++ b/crates/tor-rtcompat/src/traits.rs @@ -16,7 +16,7 @@ use std::time::{Duration, Instant, SystemTime}; /// * [`SleepProvider`] to pause a task for a given amount of time. /// * [`TcpProvider`] to launch and accept TCP connections. /// * [`TlsProvider`] to launch TLS connections. -/// * [`SpawnBlocking`] to block on a future and run it to completion +/// * [`BlockOn`] to block on a future and run it to completion /// (This may become optional in the future, if/when we add WASM /// support). /// @@ -29,7 +29,7 @@ pub trait Runtime: Sync + Send + Spawn - + SpawnBlocking + + BlockOn + Clone + SleepProvider + TcpProvider @@ -42,7 +42,7 @@ impl Runtime for T where T: Sync + Send + Spawn - + SpawnBlocking + + BlockOn + Clone + SleepProvider + TcpProvider @@ -105,7 +105,7 @@ pub trait SleepProvider { } /// Trait for a runtime that can block on a future. -pub trait SpawnBlocking { +pub trait BlockOn { /// Run `future` until it is ready, and return its output. fn block_on(&self, future: F) -> F::Output; } diff --git a/crates/tor-rtmock/src/net_runtime.rs b/crates/tor-rtmock/src/net_runtime.rs index 36208e6d4..bc59f7230 100644 --- a/crates/tor-rtmock/src/net_runtime.rs +++ b/crates/tor-rtmock/src/net_runtime.rs @@ -4,7 +4,7 @@ // we should make it so that more code is more shared. use crate::net::MockNetProvider; -use tor_rtcompat::{Runtime, SleepProvider, SpawnBlocking, TcpProvider, TlsProvider}; +use tor_rtcompat::{BlockOn, Runtime, SleepProvider, TcpProvider, TlsProvider}; use crate::io::LocalStream; use async_trait::async_trait; @@ -48,7 +48,7 @@ impl Spawn for MockNetRuntime { } } -impl SpawnBlocking for MockNetRuntime { +impl BlockOn for MockNetRuntime { fn block_on(&self, future: F) -> F::Output { self.runtime.block_on(future) } diff --git a/crates/tor-rtmock/src/sleep_runtime.rs b/crates/tor-rtmock/src/sleep_runtime.rs index 29cb4f7ae..59855cb49 100644 --- a/crates/tor-rtmock/src/sleep_runtime.rs +++ b/crates/tor-rtmock/src/sleep_runtime.rs @@ -1,7 +1,7 @@ //! Declare MockSleepRuntime. use crate::time::MockSleepProvider; -use tor_rtcompat::{Runtime, SleepProvider, SpawnBlocking, TcpProvider, TlsProvider}; +use tor_rtcompat::{BlockOn, Runtime, SleepProvider, TcpProvider, TlsProvider}; use async_trait::async_trait; use futures::task::{FutureObj, Spawn, SpawnError}; @@ -86,7 +86,7 @@ impl Spawn for MockSleepRuntime { } } -impl SpawnBlocking for MockSleepRuntime { +impl BlockOn for MockSleepRuntime { fn block_on(&self, future: F) -> F::Output { self.runtime.block_on(future) } From 9c043a648a1a05ae0f2fcb30990f7a8eda831e63 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 14:31:49 -0500 Subject: [PATCH 12/16] arti: be more careful to use the user-selected runtime --- Cargo.lock | 1 + crates/arti/Cargo.toml | 3 ++- crates/arti/src/main.rs | 15 +++++++++++---- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ef721349..873ce1a29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,6 +67,7 @@ dependencies = [ "arti-client", "arti-config", "async-ctrlc", + "cfg-if", "clap", "config", "futures", diff --git a/crates/arti/Cargo.toml b/crates/arti/Cargo.toml index 363ba76d3..266a8c746 100644 --- a/crates/arti/Cargo.toml +++ b/crates/arti/Cargo.toml @@ -20,7 +20,7 @@ static = [ "arti-client/static" ] journald = [ "tracing-journald" ] [dependencies] -arti-client = { package="arti-client", path = "../arti-client", version = "0.0.3"} +arti-client = { package="arti-client", path = "../arti-client", version = "0.0.3", default-features=false} tor-rtcompat = { path="../tor-rtcompat", version = "0.0.3", default-features=false } tor-socksproto = { path="../tor-socksproto", version = "0.0.3"} arti-config = { path="../arti-config", version = "0.0.3"} @@ -28,6 +28,7 @@ arti-config = { path="../arti-config", version = "0.0.3"} anyhow = "1.0.5" async-ctrlc = { version = "1.2.0", optional = true } config = { version = "0.11.0", default-features = false } +cfg-if = "1.0.0" futures = "0.3" tracing = "0.1.18" once_cell = { version = "1", optional = true } diff --git a/crates/arti/src/main.rs b/crates/arti/src/main.rs index 14693c14b..3cf8c2a8c 100644 --- a/crates/arti/src/main.rs +++ b/crates/arti/src/main.rs @@ -229,10 +229,17 @@ fn main() -> Result<()> { process::use_max_file_limit(); - #[cfg(all(feature = "async-std", not(feature = "tokio")))] - use tor_rtcompat::tokio::PreferredRuntime as ChosenRuntime; - #[cfg(feature = "tokio")] - use tor_rtcompat::tokio::PreferredRuntime as ChosenRuntime; + cfg_if::cfg_if! { + if #[cfg(all(feature="tokio", feature="native-tls"))] { + use tor_rtcompat::tokio::TokioNativeTlsRuntime as ChosenRuntime; + } else if #[cfg(all(feature="tokio", feature="rustls"))] { + use tor_rtcompat::tokio::TokioRustlsRuntime as ChosenRuntime; + } else if #[cfg(all(feature="async-std", feature="native-tls"))] { + use tor_rtcompat::tokio::TokioRustlsRuntime as ChosenRuntime; + } else if #[cfg(all(feature="async-std", feature="rustls"))] { + use tor_rtcompat::tokio::TokioRustlsRuntime as ChosenRuntime; + } + } let runtime = ChosenRuntime::create()?; From 5dcc821146542ebe7e794c306365be4117df86c2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 14:58:28 -0500 Subject: [PATCH 13/16] Fix documentation references for tor-rtcompat refactoring. --- crates/arti-client/src/lib.rs | 12 +++++++----- crates/tor-rtcompat/src/lib.rs | 12 +++++++----- crates/tor-rtcompat/src/tokio.rs | 8 ++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/crates/arti-client/src/lib.rs b/crates/arti-client/src/lib.rs index 69e3cf578..84920df08 100644 --- a/crates/arti-client/src/lib.rs +++ b/crates/arti-client/src/lib.rs @@ -118,14 +118,16 @@ //! will expect a type that implements [`tor_rtcompat::Runtime`], which can be obtained: //! //! - for Tokio: -//! - by calling [`tor_rtcompat::tokio::current_runtime`], if a Tokio reactor is already running -//! - by calling [`tor_rtcompat::tokio::create_runtime`], to start a new reactor if one is not +//! - by calling [`tor_rtcompat::tokio::PreferredRuntime::current()`], if a Tokio reactor is already running +//! - by calling [`tor_rtcompat::tokio::PreferredRuntime::create()`], to start a new reactor if one is not //! already running -//! - by manually creating a [`TokioRuntimeHandle`](tor_rtcompat::tokio::TokioRuntimeHandle) from -//! an existing Tokio runtime handle +//! - as above, but explicitly specifying [`TokioNativeTlsRuntime`](tor_rtcompat::tokio::TokioNativeTlsRuntime) +//! or [`TokioRustlsRuntime`](tor_rtcompat::tokio::TokioRustlsRuntime) in place of `PreferredRuntime`. //! - for async-std: -//! - by calling [`tor_rtcompat::async_std::current_runtime`], which will create a runtime or +//! - by calling [`tor_rtcompat::async_std::PreferredRuntime::current()`], which will create a runtime or //! retrieve the existing one, if one has already been started +//! - as above, but explicitly specifying [`AsyncStdNativeTlsRuntime`](tor_rtcompat::async_std::AsyncStdNativeTlsRuntime) +//! or [`AsyncStdRustlsRuntime`](tor_rtcompat::async_std::AsyncStdRustlsRuntime) in place of `PreferredRuntime`. //! //! //! # Feature flags diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index efcdd8abd..81ac2b9d2 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -60,11 +60,13 @@ //! * If you want to construct a default runtime that you won't be //! using for anything besides Arti, you can use [`create_runtime()`]. //! -//! * If you want to explicitly construct a runtime with a specific -//! backend, you can do so with [`async_std::create_runtime`] or -//! [`tokio::create_runtime`]. Or if you have already constructed a +//! * If you want to use a runtime with an explicitly chosen backend, +//! name its type directly as [`async_std::AsyncStdNativeTlsRuntime`], +//! [`async_std::AsyncStdRustlsRuntime`], [`tokio::TokioNativeTlsRuntime`], +//! or [`tokio::TokioRustlsRuntime`]. To construct one of these runtimes, +//! call its `create()` method. Or if you have already constructed a //! tokio runtime that you want to use, you can wrap it as a -//! [`Runtime`] explicitly with [`tokio::TokioRuntimeHandle`]. +//! [`Runtime`] explicitly with `current()`. //! //! # Cargo features //! @@ -234,7 +236,7 @@ pub fn current_user_runtime() -> std::io::Result { /// /// Tokio users may want to avoid this function and instead make a /// runtime using [`current_user_runtime()`] or -/// [`tokio::current_runtime()`]: this function always _builds_ a +/// [`tokio::PreferredRuntime::current()`]: this function always _builds_ a /// runtime, and if you already have a runtime, that isn't what you /// want with Tokio. /// diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index 7a504e56d..67b2341e5 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -24,7 +24,7 @@ pub use TokioNativeTlsRuntime as PreferredRuntime; #[cfg(all(feature = "rustls", not(feature = "native-tls")))] pub use TokioRustlsRuntime as PreferredRuntime; -/// A [`Runtime`] built around a Handle to a tokio runtime, and `native_tls`. +/// A [`Runtime`](crate::Runtime) built around a Handle to a tokio runtime, and `native_tls`. /// /// # Limitations /// @@ -42,7 +42,7 @@ pub struct TokioNativeTlsRuntime { #[cfg(feature = "native-tls")] type HandleInner = CompoundRuntime>; -/// A [`Runtime`] built around a Handle to a tokio runtime, and `rustls`. +/// A [`Runtime`](crate::Runtime) built around a Handle to a tokio runtime, and `rustls`. #[derive(Clone)] #[cfg(feature = "rustls")] pub struct TokioRustlsRuntime { @@ -107,7 +107,7 @@ impl TokioNativeTlsRuntime { /// We should never call this from inside other Arti crates, or from library /// crates that want to support multiple runtimes! This function is for /// Arti _users_ who want to wrap some existing Tokio runtime as a - /// [`Runtime`]. It is not for library crates that want to work with + /// [`Runtime`](crate::Runtime). It is not for library crates that want to work with /// multiple runtimes. /// /// Once you have a runtime returned by this function, you should just @@ -154,7 +154,7 @@ impl TokioRustlsRuntime { /// We should never call this from inside other Arti crates, or from library /// crates that want to support multiple runtimes! This function is for /// Arti _users_ who want to wrap some existing Tokio runtime as a - /// [`Runtime`]. It is not for library crates that want to work with + /// [`Runtime`](crate::Runtime). It is not for library crates that want to work with /// multiple runtimes. /// /// Once you have a runtime returned by this function, you should just From 474a46999d690b6200e5511872f1a45a728c670c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2022 15:00:04 -0500 Subject: [PATCH 14/16] CI: Fix async-std task Previously it didn't enable any TLS provider, since we made native-tls optional a few commits ago. Now it enables rustls, so that rustls also gets a quick check along with async-std. I've also switched this test to use "cargo clippy" in place of "cargo test" because it's a strict superset. --- .gitlab-ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e89edb256..9ab5ae38b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -28,11 +28,12 @@ rust-latest: - target/x86_64-unknown-linux-gnu/debug/arti expire_in: 1 hours -rust-latest-async-std: +rust-latest-async-std-rustls: stage: build image: rust:latest script: - - cd crates/arti-client && cargo check --no-default-features --features=async-std + - rustup component add clippy + - cd crates/arti-client && cargo clippy --no-default-features --features=async-std,rustls tags: - amd64 From c4321289f24c778f0451112cf4f4c9eac8b77fad Mon Sep 17 00:00:00 2001 From: eta Date: Thu, 27 Jan 2022 15:25:57 +0000 Subject: [PATCH 15/16] Apply @eta's suggestions from review on !263 Comment-only. --- crates/tor-rtcompat/src/lib.rs | 2 +- crates/tor-rtcompat/src/tokio.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index 81ac2b9d2..5ba25d6bd 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -253,7 +253,7 @@ pub fn create_runtime() -> std::io::Result { /// Helpers for test_with_all_runtimes pub mod testing__ { /// A trait for an object that might represent a test failure, or which - /// might just be `(). + /// might just be `()`. pub trait TestOutcome { /// Abort if the test has failed. fn check_ok(&self); diff --git a/crates/tor-rtcompat/src/tokio.rs b/crates/tor-rtcompat/src/tokio.rs index 67b2341e5..9a92dd989 100644 --- a/crates/tor-rtcompat/src/tokio.rs +++ b/crates/tor-rtcompat/src/tokio.rs @@ -99,7 +99,7 @@ impl TokioNativeTlsRuntime { }) } - /// Return a [`TokioNativeTlsRuntime`] wrapping the currently running + /// Return a [`TokioNativeTlsRuntime`] wrapping the currently running /// Tokio runtime. /// /// # Usage note From 9f3ad85d75ac7c5a1edf6be1be1008cd1015ee08 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 27 Jan 2022 10:31:45 -0500 Subject: [PATCH 16/16] Document why {current,create}_runtime are type-erased --- crates/tor-rtcompat/src/lib.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/tor-rtcompat/src/lib.rs b/crates/tor-rtcompat/src/lib.rs index 5ba25d6bd..8aa517dfc 100644 --- a/crates/tor-rtcompat/src/lib.rs +++ b/crates/tor-rtcompat/src/lib.rs @@ -220,6 +220,12 @@ pub use preferred_backend_mod::PreferredRuntime; /// /// Once you have a runtime returned by this function, you should /// just create more handles to it via [`Clone`]. +/// +/// This function returns a type-erased `impl Runtime` rather than a specific +/// runtime implementation, so that you can be sure that your code doesn't +/// depend on any runtime-specific features. If that's not what you want, you +/// can call [`PreferredRuntime::current`], or the `create` function on some +/// specific runtime in the `tokio` or `async_std` modules. #[cfg(all( any(feature = "native-tls", feature = "rustls"), any(feature = "async-std", feature = "tokio") @@ -231,17 +237,21 @@ pub fn current_user_runtime() -> std::io::Result { /// Return a new instance of the default [`Runtime`]. /// /// Generally you should call this function at most once, and then use -/// [`Clone::clone()`] to create additional references to that -/// runtime. +/// [`Clone::clone()`] to create additional references to that runtime. /// -/// Tokio users may want to avoid this function and instead make a -/// runtime using [`current_user_runtime()`] or -/// [`tokio::PreferredRuntime::current()`]: this function always _builds_ a -/// runtime, and if you already have a runtime, that isn't what you -/// want with Tokio. +/// Tokio users may want to avoid this function and instead make a runtime using +/// [`current_user_runtime()`] or [`tokio::PreferredRuntime::current()`]: this +/// function always _builds_ a runtime, and if you already have a runtime, that +/// isn't what you want with Tokio. /// -/// If you need more fine-grained control over a runtime, you can -/// create it using an appropriate builder type or function. +/// If you need more fine-grained control over a runtime, you can create it +/// using an appropriate builder type or function. +/// +/// This function returns a type-erased `impl Runtime` rather than a specific +/// runtime implementation, so that you can be sure that your code doesn't +/// depend on any runtime-specific features. If that's not what you want, you +/// can call [`PreferredRuntime::create`], or the `create` function on some +/// specific runtime in the `tokio` or `async_std` modules. #[cfg(all( any(feature = "native-tls", feature = "rustls"), any(feature = "async-std", feature = "tokio")