From 297d9c13a09a47f4a3123b90fca5deddb78b6735 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 31 Aug 2021 19:33:27 +0200 Subject: [PATCH 1/4] fix(engine-dylib): do not keep temp file and delete it automatically #2501 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- lib/engine-dylib/src/artifact.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/engine-dylib/src/artifact.rs b/lib/engine-dylib/src/artifact.rs index 38c1603dee3..deb6c9cdefc 100644 --- a/lib/engine-dylib/src/artifact.rs +++ b/lib/engine-dylib/src/artifact.rs @@ -58,6 +58,12 @@ pub struct DylibArtifact { frame_info_registration: Mutex>, } +impl Drop for DylibArtifact { + fn drop(&mut self) { + std::fs::remove_file(&self.dylib_path).expect("cannot delete the temporary artifact"); + } +} + fn to_compile_error(err: impl Error) -> CompileError { CompileError::Codegen(err.to_string()) } @@ -231,7 +237,7 @@ impl DylibArtifact { // Re-open it. let (mut file, filepath) = file.keep().map_err(to_compile_error)?; - file.write(&obj_bytes).map_err(to_compile_error)?; + file.write_all(&obj_bytes).map_err(to_compile_error)?; filepath } None => { @@ -261,7 +267,7 @@ impl DylibArtifact { let (mut file, filepath) = file.keep().map_err(to_compile_error)?; let obj_bytes = obj.write().map_err(to_compile_error)?; - file.write(&obj_bytes).map_err(to_compile_error)?; + file.write_all(&obj_bytes).map_err(to_compile_error)?; filepath } }; From efc84e1ff468013dd753579168e2cd50c8ef8a5b Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 1 Sep 2021 12:08:38 +0200 Subject: [PATCH 2/4] fix(engine-dylib): do not delete temp file in every scenarios Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- lib/engine-dylib/src/artifact.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/engine-dylib/src/artifact.rs b/lib/engine-dylib/src/artifact.rs index deb6c9cdefc..ca7d2f4c657 100644 --- a/lib/engine-dylib/src/artifact.rs +++ b/lib/engine-dylib/src/artifact.rs @@ -48,6 +48,7 @@ use wasmer_vm::{ #[derive(MemoryUsage)] pub struct DylibArtifact { dylib_path: PathBuf, + is_temporary: bool, metadata: ModuleMetadata, finished_functions: BoxedSlice, #[loupe(skip)] @@ -60,7 +61,9 @@ pub struct DylibArtifact { impl Drop for DylibArtifact { fn drop(&mut self) { - std::fs::remove_file(&self.dylib_path).expect("cannot delete the temporary artifact"); + if self.is_temporary { + std::fs::remove_file(&self.dylib_path).expect("cannot delete the temporary artifact"); + } } } @@ -374,12 +377,15 @@ impl DylibArtifact { trace!("gcc command result {:?}", output); - if is_cross_compiling { + let mut artifact = if is_cross_compiling { Self::from_parts_crosscompiled(metadata, output_filepath) } else { let lib = unsafe { Library::new(&output_filepath).map_err(to_compile_error)? }; Self::from_parts(&mut engine_inner, metadata, output_filepath, lib) - } + }?; + artifact.is_temporary = true; + + Ok(artifact) } /// Get the default extension when serializing this artifact @@ -406,6 +412,7 @@ impl DylibArtifact { let signatures: PrimaryMap = PrimaryMap::new(); Ok(Self { dylib_path, + is_temporary: false, metadata, finished_functions: finished_functions.into_boxed_slice(), finished_function_call_trampolines: finished_function_call_trampolines @@ -511,6 +518,7 @@ impl DylibArtifact { Ok(Self { dylib_path, + is_temporary: false, metadata, finished_functions: finished_functions.into_boxed_slice(), finished_function_call_trampolines: finished_function_call_trampolines @@ -552,7 +560,10 @@ impl DylibArtifact { file.write_all(&bytes)?; // We already checked for the header, so we don't need // to check again. - Self::deserialize_from_file_unchecked(&engine, &path) + let mut artifact = Self::deserialize_from_file_unchecked(&engine, &path)?; + artifact.is_temporary = true; + + Ok(artifact) } /// Deserialize a `DylibArtifact` from a file path. From 8d8ee21d50f0ba01875624c235699e8400dd6fab Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 1 Sep 2021 12:21:00 +0200 Subject: [PATCH 3/4] fix(engine-dylib): display error log if we can't delete the tmp file Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- lib/engine-dylib/src/artifact.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/engine-dylib/src/artifact.rs b/lib/engine-dylib/src/artifact.rs index ca7d2f4c657..a6794b9925f 100644 --- a/lib/engine-dylib/src/artifact.rs +++ b/lib/engine-dylib/src/artifact.rs @@ -13,6 +13,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::{Arc, Mutex}; use tempfile::NamedTempFile; +use tracing::log::error; #[cfg(feature = "compiler")] use tracing::trace; use wasmer_compiler::{ @@ -62,7 +63,9 @@ pub struct DylibArtifact { impl Drop for DylibArtifact { fn drop(&mut self) { if self.is_temporary { - std::fs::remove_file(&self.dylib_path).expect("cannot delete the temporary artifact"); + if let Err(err) = std::fs::remove_file(&self.dylib_path) { + error!("cannot delete the temporary dylib artifact: {}", err); + } } } } From a0dbc99753e3391575f3377e200e25ef558d6ef6 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 1 Sep 2021 14:19:48 +0200 Subject: [PATCH 4/4] fix(engine-dylib): add log feature for tracing in engine-dylib Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- lib/engine-dylib/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/engine-dylib/Cargo.toml b/lib/engine-dylib/Cargo.toml index e3b6350cdbd..1b356e07020 100644 --- a/lib/engine-dylib/Cargo.toml +++ b/lib/engine-dylib/Cargo.toml @@ -18,7 +18,7 @@ wasmer-engine = { path = "../engine", version = "2.0.0" } wasmer-object = { path = "../object", version = "2.0.0" } serde = { version = "1.0", features = ["derive", "rc"] } cfg-if = "1.0" -tracing = "0.1" +tracing = { version = "0.1", features = ["log"] } leb128 = "0.2" libloading = "0.7" tempfile = "3.1"