Skip to content

Commit

Permalink
bug: deployer freezes (#478)
Browse files Browse the repository at this point in the history
* bug: reduce spawning to have deployer lock up less

* refactor: don't return cargo logs
  • Loading branch information
chesedo authored Nov 17, 2022
1 parent 7b80c45 commit c3c0ced
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 59 deletions.
4 changes: 2 additions & 2 deletions cargo-shuttle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl Shuttle {
trace!("starting a local run for a service: {run_args:?}");

let (tx, rx): (crossbeam_channel::Sender<Message>, _) = crossbeam_channel::bounded(0);
tokio::spawn(async move {
tokio::task::spawn_blocking(move || {
while let Ok(message) = rx.recv() {
match message {
Message::TextLine(line) => println!("{line}"),
Expand Down Expand Up @@ -332,7 +332,7 @@ impl Shuttle {

handle.await??;

tokio::spawn(async move {
tokio::task::spawn_blocking(move || {
trace!("closing so file");
so.close().unwrap();
});
Expand Down
62 changes: 30 additions & 32 deletions deployer/src/deployment/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Queued {

let (tx, rx): (crossbeam_channel::Sender<Message>, _) = crossbeam_channel::bounded(0);
let id = self.id;
tokio::spawn(async move {
tokio::task::spawn_blocking(move || {
while let Ok(message) = rx.recv() {
trace!(?message, "received cargo message");
// TODO: change these to `info!(...)` as [valuable] support increases.
Expand Down Expand Up @@ -286,6 +286,8 @@ async fn build_deployment(
.await
.map_err(|e| Error::Build(e.into()))?;

trace!(?so_path, "got so path");

Ok(so_path)
}

Expand All @@ -297,36 +299,8 @@ async fn run_pre_deploy_tests(
let (read, write) = pipe::pipe();
let project_path = project_path.to_owned();

let handle = tokio::spawn(async move {
let config = get_config(write)?;
let manifest_path = project_path.join("Cargo.toml");

let ws = Workspace::new(&manifest_path, &config)?;

let mut compile_opts = CompileOptions::new(&config, CompileMode::Test)?;

compile_opts.build_config.message_format = MessageFormat::Json {
render_diagnostics: false,
short: false,
ansi: false,
};

let opts = TestOptions {
compile_opts,
no_run: false,
no_fail_fast: false,
};

let test_failures = cargo::ops::run_tests(&ws, &opts, &[])?;

match test_failures {
Some(failures) => Err(failures.into()),
None => Ok(()),
}
});

// This needs to be on a separate thread, else deployer will block (reason currently unknown :D)
tokio::spawn(async move {
tokio::task::spawn_blocking(move || {
for message in Message::parse_stream(read) {
match message {
Ok(message) => {
Expand All @@ -341,7 +315,31 @@ async fn run_pre_deploy_tests(
}
});

handle.await?
let config = get_config(write)?;
let manifest_path = project_path.join("Cargo.toml");

let ws = Workspace::new(&manifest_path, &config)?;

let mut compile_opts = CompileOptions::new(&config, CompileMode::Test)?;

compile_opts.build_config.message_format = MessageFormat::Json {
render_diagnostics: false,
short: false,
ansi: false,
};

let opts = TestOptions {
compile_opts,
no_run: false,
no_fail_fast: false,
};

let test_failures = cargo::ops::run_tests(&ws, &opts, &[])?;

match test_failures {
Some(failures) => Err(failures.into()),
None => Ok(()),
}
}

/// Store 'so' file in the libs folder
Expand Down Expand Up @@ -462,7 +460,7 @@ ff0e55bda1ff01000000000000000000e0079c01ff12a55500280000",
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let (tx, rx) = crossbeam_channel::unbounded();

tokio::spawn(async move { while rx.recv().is_ok() {} });
tokio::task::spawn_blocking(move || while rx.recv().is_ok() {});

let failure_project_path = root.join("tests/resources/tests-fail");
assert!(matches!(
Expand Down
1 change: 0 additions & 1 deletion deployer/src/persistence/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ fn extract_message(fields: &Value) -> Option<String> {
return Some(rendered.as_str()?.to_string());
}
}
Value::String(mes_str) => return Some(mes_str.to_string()),
_ => {}
}
}
Expand Down
43 changes: 19 additions & 24 deletions service/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl Loader {
/// 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!("loading {:?}", so_path.as_ref().to_str());
trace!(so_path = so_path.as_ref().to_str(), "loading .so path");
unsafe {
let lib = Library::new(so_path).map_err(LoaderError::Load)?;

Expand Down Expand Up @@ -110,29 +110,8 @@ pub async fn build_crate(
let (read, write) = pipe::pipe();
let project_path = project_path.to_owned();

let handle = tokio::spawn(async move {
trace!("started thread to build crate");
let config = get_config(write)?;
let manifest_path = project_path.join("Cargo.toml");
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);
check_version(summary)?;
check_no_panic(&ws)?;

let opts = get_compile_options(&config, release_mode)?;
let compilation = compile(&ws, &opts);

Ok(compilation?.cdylibs[0].path.clone())
});

// This needs to be on a separate thread, else deployer will block (reason currently unknown :D)
tokio::spawn(async move {
tokio::task::spawn_blocking(move || {
trace!("started thread to to capture build output stream");
for message in Message::parse_stream(read) {
trace!(?message, "parsed cargo message");
Expand All @@ -149,7 +128,23 @@ pub async fn build_crate(
}
});

handle.await?
let config = get_config(write)?;
let manifest_path = project_path.join("Cargo.toml");
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);
check_version(summary)?;
check_no_panic(&ws)?;

let opts = get_compile_options(&config, release_mode)?;
let compilation = compile(&ws, &opts);

Ok(compilation?.cdylibs[0].path.clone())
}

/// Get the default compile config with output redirected to writer
Expand Down

0 comments on commit c3c0ced

Please sign in to comment.