feat: trim the service loader, unpin tokio

runtime/Cargo.toml

@@ -17,7 +17,7 @@ clap ={ version = "4.0.18", features = ["derive"] }
 hyper = { version = "0.14.23", features = ["server"] }
 rmp-serde = { version = "1.1.1" }
 thiserror = { workspace = true }
-tokio = { version = "=1.22.0", features = ["full"] }
+tokio = { version = "1.22.0", features = ["full"] }
 tokio-stream = "0.1.11"
 tonic = { workspace = true }
 tracing = { workspace = true }

service/Cargo.toml

@@ -22,7 +22,6 @@ chrono = { workspace = true }
 crossbeam-channel = "0.5.6"
 futures = { version = "0.3.25", features = ["std"] }
 hyper = { version = "0.14.23", features = ["server", "tcp", "http1"], optional = true }
-libloading = { version = "0.7.4", optional = true }
 num_cpus = { version = "1.14.0", optional = true }
 pipe = "0.4.0"
 poem = { version = "1.3.49", optional = true }
@@ -34,7 +33,7 @@ poise = { version = "0.5.2", optional = true }
 thiserror = { workspace = true }
 thruster = { version = "1.3.0", optional = true }
 tide = { version = "0.16.0", optional = true }
-tokio = { version = "=1.22.0", features = ["rt", "rt-multi-thread", "sync"] }
+tokio = { version = "1.22.0", features = ["sync"] }
 tower = { version = "0.4.13", features = ["make"], optional = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true, features = ["env-filter"] }
@@ -66,7 +65,7 @@ uuid = { workspace = true, features = ["v4"] }
 default = ["codegen"]
 
 codegen = ["shuttle-codegen/frameworks"]
-loader = ["cargo", "libloading"]
+loader = ["cargo"]
 
 web-actix-web = ["actix-web", "num_cpus"]
 web-axum = ["axum"]

service/src/lib.rs

@@ -85,7 +85,6 @@
 //!
 //! ```bash
 //! $ cargo shuttle project new
-//! $ cargo shuttle project status // until the project is "ready"
 //! ```
 //!
 //! Then, deploy the service with:
@@ -211,16 +210,13 @@
 //!
 
 use std::collections::BTreeMap;
-use std::future::Future;
 use std::net::SocketAddr;
 use std::path::PathBuf;
-use std::pin::Pin;
 
 pub use async_trait::async_trait;
 
 // Pub uses by `codegen`
 pub use anyhow::Context;
-pub use tokio::runtime::Runtime;
 pub use tracing;
 pub use tracing_subscriber;
 
@@ -286,7 +282,6 @@ extern crate shuttle_codegen;
 ///
 /// More [shuttle managed resources can be found here](https://github.com/shuttle-hq/shuttle/tree/main/resources)
 pub use shuttle_codegen::main;
-use tokio::task::JoinHandle;
 
 #[cfg(feature = "loader")]
 pub mod loader;
@@ -329,9 +324,6 @@ pub trait Factory: Send + Sync {
 /// You may want to create your own managed resource by implementing this trait for some builder `B` to construct resource `T`. [`Factory`] can be used to provision resources
 /// on shuttle's servers if your resource will need any.
 ///
-/// The biggest thing to look out for is that your resource object might panic when it crosses the boundary between the shuttle's backend runtime and the runtime
-/// of services. These resources should be created on the passed in `runtime` for this trait to prevent these panics.
-///
 /// Your resource will be available on a [shuttle_service::main][main] function as follow:
 /// ```
 /// #[shuttle_service::main]
@@ -371,7 +363,6 @@ pub trait Factory: Send + Sync {
 ///     async fn build(
 ///         self,
 ///         factory: &mut dyn Factory,
-///         _runtime: &Runtime,
 ///     ) -> Result<Resource, shuttle_service::Error> {
 ///         Ok(Resource { name: self.name })
 ///     }
@@ -392,87 +383,17 @@ pub trait ResourceBuilder<T> {
     async fn build(self, factory: &mut dyn Factory) -> Result<T, crate::Error>;
 }
 
-/// A tokio handle the service was started on
-pub type ServeHandle = JoinHandle<Result<(), error::Error>>;
-
 /// The core trait of the shuttle platform. Every crate deployed to shuttle needs to implement this trait.
 ///
 /// Use the [main][main] macro to expose your implementation to the deployment backend.
-//
-// TODO: our current state machine in the api crate stores this service and can move it across
-// threads (handlers) causing `Service` to need `Sync`. We should remove this restriction
 #[async_trait]
-pub trait Service: Send + Sync {
+pub trait Service: Send {
     /// This function is run exactly once on each instance of a deployment.
     ///
     /// The deployer expects this instance of [Service][Service] to bind to the passed [SocketAddr][SocketAddr].
     async fn bind(mut self, addr: SocketAddr) -> Result<(), error::Error>;
 }
 
-/// This function is generated by our codegen. It uses the factory to get other services and instantiate them on
-/// the correct tokio runtime. This function also sets the runtime logger. The output is a future where `T`
-/// should implement [Service].
-pub type StateBuilder<T> =
-    for<'a> fn(
-        &'a mut dyn Factory,
-        &'a Runtime,
-        Logger,
-    ) -> Pin<Box<dyn Future<Output = Result<T, Error>> + Send + 'a>>;
-
-/// This function is generated by codegen to ensure binding happens on the other side of the FFI and on the correct
-/// tokio runtime.
-pub type Binder = for<'a> fn(Box<dyn Service>, SocketAddr, &'a Runtime) -> ServeHandle;
-
-// Make sure every crate used in this struct has its version pinned down to prevent segmentation faults when crossing the FFI.
-// Your future self will thank you!
-// See https://github.com/shuttle-hq/shuttle/pull/348
-#[allow(dead_code)]
-pub struct Bootstrapper {
-    service: Option<Box<dyn Service>>,
-    builder: Option<StateBuilder<Box<dyn Service>>>,
-    binder: Binder,
-    runtime: Option<Runtime>,
-}
-
-impl Bootstrapper {
-    pub fn new(builder: StateBuilder<Box<dyn Service>>, binder: Binder, runtime: Runtime) -> Self {
-        Self {
-            service: None,
-            builder: Some(builder),
-            binder,
-            runtime: Some(runtime),
-        }
-    }
-
-    #[cfg(feature = "loader")]
-    async fn bootstrap(&mut self, factory: &mut dyn Factory, logger: Logger) -> Result<(), Error> {
-        if let Some(builder) = self.builder.take() {
-            let service = builder(factory, self.runtime.as_ref().unwrap(), logger).await?;
-            self.service = Some(service);
-        }
-
-        Ok(())
-    }
-
-    #[cfg(feature = "loader")]
-    fn into_handle(mut self, addr: SocketAddr) -> Result<ServeHandle, Error> {
-        let service = self.service.take().expect("service has already been bound");
-
-        let handle = (self.binder)(service, addr, self.runtime.as_ref().unwrap());
-
-        Ok(handle)
-    }
-}
-
-impl Drop for Bootstrapper {
-    fn drop(&mut self) {
-        if let Some(runtime) = self.runtime.take() {
-            // TODO: find a way to drop the runtime
-            std::mem::forget(runtime);
-        }
-    }
-}
-
 #[cfg(feature = "web-rocket")]
 #[async_trait]
 impl Service for rocket::Rocket<rocket::Build> {

service/src/loader.rs

@@ -1,106 +1,18 @@
-use std::any::Any;
-use std::ffi::OsStr;
-use std::net::SocketAddr;
-use std::panic::AssertUnwindSafe;
 use std::path::{Path, PathBuf};
 
 use anyhow::{anyhow, Context};
 use cargo::core::compiler::{CompileKind, CompileMode, CompileTarget, MessageFormat};
-use cargo::core::{Manifest, PackageId, Shell, Summary, Verbosity, Workspace};
+use cargo::core::{Shell, Summary, Verbosity, Workspace};
 use cargo::ops::{clean, compile, CleanOptions, CompileOptions};
 use cargo::util::interning::InternedString;
 use cargo::util::{homedir, ToSemver};
 use cargo::Config;
 use cargo_metadata::Message;
 use crossbeam_channel::Sender;
-use libloading::{Library, Symbol};
 use pipe::PipeWriter;
-use thiserror::Error as ThisError;
 use tracing::{error, trace};
 
-use futures::FutureExt;
-use uuid::Uuid;
-
-use crate::error::CustomError;
-use crate::{logger, Bootstrapper, NAME, NEXT_NAME, VERSION};
-use crate::{Error, Factory, ServeHandle};
-
-const ENTRYPOINT_SYMBOL_NAME: &[u8] = b"_create_service\0";
-
-type CreateService = unsafe extern "C" fn() -> *mut Bootstrapper;
-
-#[derive(Debug, ThisError)]
-pub enum LoaderError {
-    #[error("failed to load library: {0}")]
-    Load(libloading::Error),
-    #[error("failed to find the shuttle entrypoint. Did you use the provided shuttle macros?")]
-    GetEntrypoint(libloading::Error),
-}
-
-pub type LoadedService = (ServeHandle, Library);
-
-pub struct Loader {
-    bootstrapper: Bootstrapper,
-    so: Library,
-}
-
-impl Loader {
-    /// Dynamically load from a `.so` file a value of a type implementing the
-    /// [`Service`][crate::Service] trait. Relies on the `.so` library having an `extern "C"`
-    /// function called `ENTRYPOINT_SYMBOL_NAME`, likely automatically generated
-    /// using the [`shuttle_service::main`][crate::main] macro.
-    pub fn from_so_file<P: AsRef<OsStr>>(so_path: P) -> Result<Self, LoaderError> {
-        trace!(so_path = so_path.as_ref().to_str(), "loading .so path");
-        unsafe {
-            let lib = Library::new(so_path).map_err(LoaderError::Load)?;
-
-            let entrypoint: Symbol<CreateService> = lib
-                .get(ENTRYPOINT_SYMBOL_NAME)
-                .map_err(LoaderError::GetEntrypoint)?;
-            let raw = entrypoint();
-
-            Ok(Self {
-                bootstrapper: *Box::from_raw(raw),
-                so: lib,
-            })
-        }
-    }
-
-    pub async fn load(
-        self,
-        factory: &mut dyn Factory,
-        addr: SocketAddr,
-        logger: logger::Logger,
-    ) -> Result<LoadedService, Error> {
-        trace!("loading service");
-
-        let mut bootstrapper = self.bootstrapper;
-
-        AssertUnwindSafe(bootstrapper.bootstrap(factory, logger))
-            .catch_unwind()
-            .await
-            .map_err(|e| Error::BuildPanic(map_any_to_panic_string(e)))??;
-
-        trace!("bootstrapping done");
-
-        // Start service on this side of the FFI
-        let handle = tokio::spawn(async move {
-            bootstrapper.into_handle(addr)?.await.map_err(|e| {
-                if e.is_panic() {
-                    let mes = e.into_panic();
-
-                    Error::BindPanic(map_any_to_panic_string(mes))
-                } else {
-                    Error::Custom(CustomError::new(e))
-                }
-            })?
-        });
-
-        trace!("creating handle done");
-
-        Ok((handle, self.so))
-    }
-}
+use crate::{NAME, NEXT_NAME, VERSION};
 
 /// How to run/build the project
 pub enum Runtime {
@@ -110,7 +22,6 @@ pub enum Runtime {
 
 /// Given a project directory path, builds the crate
 pub async fn build_crate(
-    deployment_id: Uuid,
     project_path: &Path,
     release_mode: bool,
     tx: Sender<Message>,
@@ -141,11 +52,8 @@ pub async fn build_crate(
     let mut ws = Workspace::new(&manifest_path, &config)?;
 
     let current = ws.current_mut().map_err(|_| anyhow!("A Shuttle project cannot have a virtual manifest file - please ensure your Cargo.toml file specifies it as a library."))?;
-    let manifest = current.manifest_mut();
-    ensure_cdylib(manifest)?;
 
     let summary = current.manifest_mut().summary_mut();
-    make_name_unique(summary, deployment_id);
 
     let is_next = is_next(summary);
     if !is_next {
@@ -259,44 +167,6 @@ fn get_compile_options(
     Ok(opts)
 }
 
-/// Make sure "cdylib" is set, else set it if possible
-fn ensure_cdylib(manifest: &mut Manifest) -> anyhow::Result<()> {
-    if let Some(target) = manifest
-        .targets_mut()
-        .iter_mut()
-        .find(|target| target.is_lib())
-    {
-        if !target.is_cdylib() {
-            *target = cargo::core::manifest::Target::lib_target(
-                target.name(),
-                vec![cargo::core::compiler::CrateType::Cdylib],
-                target.src_path().path().unwrap().to_path_buf(),
-                target.edition(),
-            );
-        }
-
-        Ok(())
-    } else {
-        Err(anyhow!(
-            "Your Shuttle project must be a library. Please add `[lib]` to your Cargo.toml file."
-        ))
-    }
-}
-
-/// Ensure name is unique. Without this `tracing`/`log` crashes because the global subscriber is somehow "already set"
-// TODO: remove this when getting rid of the FFI
-fn make_name_unique(summary: &mut Summary, deployment_id: Uuid) {
-    let old_package_id = summary.package_id();
-    *summary = summary.clone().override_id(
-        PackageId::new(
-            format!("{}-{deployment_id}", old_package_id.name()),
-            old_package_id.version(),
-            old_package_id.source_id(),
-        )
-        .unwrap(),
-    );
-}
-
 fn is_next(summary: &Summary) -> bool {
     summary
         .dependencies()
@@ -339,23 +209,3 @@ fn check_no_panic(ws: &Workspace) -> anyhow::Result<()> {
 
     Ok(())
 }
-
-fn map_any_to_panic_string(a: Box<dyn Any>) -> String {
-    a.downcast_ref::<&str>()
-        .map(|x| x.to_string())
-        .unwrap_or_else(|| "<no panic message>".to_string())
-}
-
-#[cfg(test)]
-mod tests {
-    mod from_so_file {
-        use crate::loader::{Loader, LoaderError};
-
-        #[test]
-        fn invalid() {
-            let result = Loader::from_so_file("invalid.so");
-
-            assert!(matches!(result, Err(LoaderError::Load(_))));
-        }
-    }
-}