Fix a deadlock in TokioComponent.

The components are accessed by a lock on application state.  When some command
calls block_on to enter an async context, it obtained a write lock on the
entire application state.  This meant that if the application state were
accessed later in an async context, a deadlock would occur.  Instead the
TokioComponent holds an Option<Runtime> now, so that before calling block_on,
the caller can .take() the runtime and release the lock.  Since we only ever
enter an async context once, it's not a problem that the component is then
missing its runtime, as once we are inside of a task we can access the runtime.

zebrad/src/application.rs

@@ -3,7 +3,7 @@
 use crate::{commands::ZebradCmd, config::ZebradConfig};
 use abscissa_core::{
     application::{self, AppCell},
-    config, trace, Application, Component, EntryPoint, FrameworkError, StandardPaths,
+    config, trace, Application, EntryPoint, FrameworkError, StandardPaths,
 };
 
 /// Application state

zebrad/src/commands/connect.rs

@@ -26,13 +26,18 @@ impl Runnable for ConnectCmd {
         info!(connect.addr = ?self.addr);
 
         use crate::components::tokio::TokioComponent;
-        let _ = app_writer()
+        let rt = app_writer()
             .state_mut()
             .components
             .get_downcast_mut::<TokioComponent>()
             .expect("TokioComponent should be available")
             .rt
-            .block_on(self.connect());
+            .take();
+
+        rt.expect("runtime should not already be taken")
+            .block_on(self.connect())
+            // Surface any error that occurred executing the future.
+            .unwrap();
     }
 }
 
@@ -44,11 +49,9 @@ impl ConnectCmd {
         use tower::{buffer::Buffer, service_fn, Service, ServiceExt};
 
         let node = Buffer::new(
-            service_fn(|req| {
+            service_fn(|req| async move {
-                async move {
+                info!(?req);
-                    info!(?req);
+                Ok::<Response, Error>(Response::Ok)
-                    Ok::<Response, Error>(Response::Ok)
-                }
             }),
             1,
         );

zebrad/src/commands/seed.rs

@@ -112,13 +112,18 @@ impl Runnable for SeedCmd {
     fn run(&self) {
         use crate::components::tokio::TokioComponent;
 
-        let _ = app_writer()
+        let rt = app_writer()
             .state_mut()
             .components
             .get_downcast_mut::<TokioComponent>()
             .expect("TokioComponent should be available")
             .rt
-            .block_on(self.seed());
+            .take();
+
+        rt.expect("runtime should not already be taken")
+            .block_on(self.seed())
+            // Surface any error that occurred executing the future.
+            .unwrap();
     }
 }
 

zebrad/src/commands/start.rs

@@ -40,12 +40,15 @@ impl Runnable for StartCmd {
 
         use crate::components::tokio::TokioComponent;
 
-        app_writer()
+        let rt = app_writer()
             .state_mut()
             .components
             .get_downcast_mut::<TokioComponent>()
             .expect("TokioComponent should be available")
             .rt
+            .take();
+
+        rt.expect("runtime should not already be taken")
             .block_on(future::pending::<()>());
     }
 }

zebrad/src/components/tokio.rs

@@ -5,15 +5,21 @@ use abscissa_core::{Component, FrameworkError};
 use tokio::runtime::Runtime;
 
 /// An Abscissa component which owns a Tokio runtime.
+///
+/// The runtime is stored as an `Option` so that when it's time to enter an async
+/// context by calling `block_on` with a "root future", the runtime can be taken
+/// independently of Abscissa's component locking system. Otherwise whatever
+/// calls `block_on` holds an application lock for the entire lifetime of the
+/// async context.
 #[derive(Component, Debug)]
 pub struct TokioComponent {
-    pub rt: Runtime,
+    pub rt: Option<Runtime>,
 }
 
 impl TokioComponent {
     pub fn new() -> Result<Self, FrameworkError> {
         Ok(Self {
-            rt: Runtime::new().unwrap(),
+            rt: Some(Runtime::new().unwrap()),
         })
     }
 }

zebrad/src/components/tracing.rs

@@ -47,23 +47,27 @@ impl TracingEndpoint {
             .parse()
             .expect("Hardcoded address should be parseable");
 
-        tokio_component.rt.spawn(async move {
+        tokio_component
-            // try_bind uses the tokio runtime, so we
+            .rt
-            // need to construct it inside the task.
+            .as_ref()
-            let server = match Server::try_bind(&addr) {
+            .expect("runtime should not be taken")
-                Ok(s) => s,
+            .spawn(async move {
-                Err(e) => {
+                // try_bind uses the tokio runtime, so we
-                    error!("Could not open tracing endpoint listener");
+                // need to construct it inside the task.
-                    error!("Error: {}", e);
+                let server = match Server::try_bind(&addr) {
-                    return;
+                    Ok(s) => s,
+                    Err(e) => {
+                        error!("Could not open tracing endpoint listener");
+                        error!("Error: {}", e);
+                        return;
+                    }
                 }
-            }
+                .serve(service);
-            .serve(service);
 
-            if let Err(e) = server.await {
+                if let Err(e) = server.await {
-                error!("Server error: {}", e);
+                    error!("Server error: {}", e);
-            }
+                }
-        });
+            });
 
         Ok(())
     }

zebrad/src/prelude.rs

@@ -6,10 +6,3 @@ pub use crate::application::{app_config, app_reader, app_writer};
 
 /// Commonly used Abscissa traits
 pub use abscissa_core::{Application, Command, Runnable};
-
-/// Type alias to make working with tower traits easier.
-///
-/// Note: the 'static lifetime bound means that the *type* cannot have any
-/// non-'static lifetimes, (e.g., when a type contains a borrow and is
-/// parameterized by 'a), *not* that the object itself has 'static lifetime.
-pub(crate) use abscissa_core::error::BoxError;