Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: trim the service loader, unpin tokio #681

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cargo-shuttle/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ path = "../resources/secrets"

[dependencies.shuttle-service]
workspace = true
features = ["loader"]
features = ["builder"]

[features]
vendored-openssl = ["openssl/vendored"]
Expand Down
7 changes: 3 additions & 4 deletions cargo-shuttle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use git2::{Repository, StatusOptions};
use ignore::overrides::OverrideBuilder;
use ignore::WalkBuilder;
use shuttle_common::models::{project, secret};
use shuttle_service::loader::{build_crate, Runtime};
use shuttle_service::builder::{build_crate, Runtime};
use std::fmt::Write;
use strum::IntoEnumIterator;
use tar::Builder;
Expand Down Expand Up @@ -376,15 +376,14 @@ impl Shuttle {
});

let working_directory = self.ctx.working_directory();
let id = Default::default();

trace!("building project");
println!(
"{:>12} {}",
"Building".bold().green(),
working_directory.display()
);
let runtime = build_crate(id, working_directory, false, tx).await?;
let runtime = build_crate(working_directory, false, tx).await?;

trace!("loading secrets");
let secrets_path = working_directory.join("Secrets.toml");
Expand Down Expand Up @@ -515,7 +514,7 @@ impl Shuttle {
let addr = SocketAddr::new(addr, run_args.port);

let start_request = StartRequest {
deployment_id: id.as_bytes().to_vec(),
deployment_id: Uuid::default().as_bytes().to_vec(),
ip: addr.to_string(),
};

Expand Down
2 changes: 1 addition & 1 deletion deployer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ workspace = true

[dependencies.shuttle-service]
workspace = true
features = ["loader"]
features = ["builder"]

[dev-dependencies]
ctor = "0.1.26"
Expand Down
9 changes: 4 additions & 5 deletions deployer/src/deployment/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use chrono::Utc;
use crossbeam_channel::Sender;
use opentelemetry::global;
use serde_json::json;
use shuttle_service::loader::{build_crate, get_config, Runtime};
use shuttle_service::builder::{build_crate, get_config, Runtime};
use tokio::time::{sleep, timeout};
use tracing::{debug, debug_span, error, info, instrument, trace, warn, Instrument, Span};
use tracing_opentelemetry::OpenTelemetrySpanExt;
Expand Down Expand Up @@ -206,7 +206,7 @@ impl Queued {
});

let project_path = project_path.canonicalize()?;
let runtime = build_deployment(self.id, &project_path, tx.clone()).await?;
let runtime = build_deployment(&project_path, tx.clone()).await?;

if self.will_run_tests {
info!(
Expand Down Expand Up @@ -321,11 +321,10 @@ async fn extract_tar_gz_data(data: impl Read, dest: impl AsRef<Path>) -> Result<

#[instrument(skip(project_path, tx))]
async fn build_deployment(
deployment_id: Uuid,
project_path: &Path,
tx: crossbeam_channel::Sender<Message>,
) -> Result<Runtime> {
build_crate(deployment_id, project_path, true, tx)
build_crate(project_path, true, tx)
.await
.map_err(|e| Error::Build(e.into()))
}
Expand Down Expand Up @@ -413,7 +412,7 @@ mod tests {
use std::{collections::BTreeMap, fs::File, io::Write, path::Path};

use shuttle_common::storage_manager::ArtifactsStorageManager;
use shuttle_service::loader::Runtime;
use shuttle_service::builder::Runtime;
use tempdir::TempDir;
use tokio::fs;
use uuid::Uuid;
Expand Down
2 changes: 1 addition & 1 deletion deployer/src/handlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use shuttle_common::models::secret;
use shuttle_common::project::ProjectName;
use shuttle_common::storage_manager::StorageManager;
use shuttle_common::LogItem;
use shuttle_service::loader::clean_crate;
use shuttle_service::builder::clean_crate;
use tower_http::auth::RequireAuthorizationLayer;
use tower_http::trace::TraceLayer;
use tracing::{debug, debug_span, error, field, instrument, trace, Span};
Expand Down
2 changes: 1 addition & 1 deletion resources/static-folder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ shuttle-service = { path = "../../service", version = "0.8.0", default-features

[dev-dependencies]
tempdir = "0.3.7"
tokio = { version = "1.19.2", features = ["macros"] }
tokio = { version = "1.19.2", features = ["macros", "rt"] }
4 changes: 2 additions & 2 deletions runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,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 }
Expand All @@ -46,4 +46,4 @@ workspace = true

[dependencies.shuttle-service]
workspace = true
features = ["loader", "web-rocket"] # TODO: remove web-rocket
features = ["builder", "web-rocket"] # TODO: remove web-rocket
7 changes: 3 additions & 4 deletions service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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"] }
Expand All @@ -59,14 +58,14 @@ features = ["tracing", "service"]
[dev-dependencies]
portpicker = "0.1.1"
sqlx = { version = "0.6.2", features = ["runtime-tokio-native-tls", "postgres"] }
tokio = { version = "1.22.0", features = ["macros"] }
tokio = { version = "1.22.0", features = ["macros", "rt"] }
uuid = { workspace = true, features = ["v4"] }

[features]
default = ["codegen"]

codegen = ["shuttle-codegen/frameworks"]
loader = ["cargo", "libloading"]
builder = ["cargo"]

web-actix-web = ["actix-web", "num_cpus"]
web-axum = ["axum"]
Expand Down
156 changes: 3 additions & 153 deletions service/src/loader.rs → service/src/builder.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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>,
Expand Down Expand Up @@ -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 {
Expand All @@ -156,7 +64,7 @@ pub async fn build_crate(
let opts = get_compile_options(&config, release_mode, is_next)?;
let compilation = compile(&ws, &opts);

let path = compilation?.cdylibs[0].path.clone();
let path = compilation?.binaries[0].path.clone();
Ok(if is_next {
Runtime::Next(path)
} else {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(_))));
}
}
}
Loading