From 920c9491da50c6f048531cc8d0730dbe7e5794c6 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 30 Jul 2021 10:53:28 +0200 Subject: [PATCH 1/6] fix: Avoid serializing minidumps over procspawn --- .../src/services/symbolication.rs | 101 +++++++++--------- 1 file changed, 51 insertions(+), 50 deletions(-) diff --git a/crates/symbolicator/src/services/symbolication.rs b/crates/symbolicator/src/services/symbolication.rs index f259ecb21..7d97d7ff0 100644 --- a/crates/symbolicator/src/services/symbolication.rs +++ b/crates/symbolicator/src/services/symbolication.rs @@ -27,6 +27,7 @@ use symbolic::minidump::cfi::CfiCache; use symbolic::minidump::processor::{ CodeModule, CodeModuleId, FrameTrust, ProcessMinidumpError, ProcessState, RegVal, }; +use tempfile::NamedTempFile; use thiserror::Error; use crate::cache::CacheStatus; @@ -1692,8 +1693,6 @@ impl SymbolicationActor { handle: procspawn::JoinHandle, E>>, timeout: Duration, metric: &str, - minidump: &[u8], - minidump_cache: crate::cache::Cache, ) -> Result where T: Serialize + DeserializeOwned, @@ -1715,48 +1714,11 @@ impl SymbolicationActor { "unknown" }; metric!(counter(metric) += 1, "reason" => reason); - if !perr.is_timeout() { - Self::save_minidump(minidump, minidump_cache) - .map_err(|e| log::error!("Failed to save minidump {:?}", &e)) - .map(|r| { - if let Some(path) = r { - sentry::configure_scope(|scope| { - scope.set_extra( - "crashed_minidump", - sentry::protocol::Value::String( - path.to_string_lossy().to_string(), - ), - ); - }); - } - }) - .ok(); - } Err(anyhow::Error::new(perr)) } } } - /// Save a minidump to temporary location. - fn save_minidump( - minidump: &[u8], - failed_cache: crate::cache::Cache, - ) -> anyhow::Result> { - if let Some(dir) = failed_cache.cache_dir() { - std::fs::create_dir_all(dir)?; - let tmp = tempfile::Builder::new() - .prefix("minidump") - .suffix(".dmp") - .tempfile_in(dir)?; - tmp.as_file().write_all(&*minidump)?; - let (_file, path) = tmp.keep().map_err(|e| e.error)?; - Ok(Some(path)) - } else { - log::debug!("No diagnostics retention configured, not saving minidump"); - Ok(None) - } - } - async fn load_cfi_caches( &self, scope: Scope, @@ -1808,21 +1770,21 @@ impl SymbolicationActor { /// modules. async fn stackwalk_minidump_with_cfi( &self, - minidump: Bytes, + minidump_file: &NamedTempFile, cfi_caches: &CfiCacheModules, - ) -> Result { + ) -> anyhow::Result { let pool = self.spawnpool.clone(); - let diagnostics_cache = self.diagnostics_cache.clone(); let cfi_caches = cfi_caches.for_processing(); + let minidump_path = minidump_file.path().to_owned(); let lazy = async move { let spawn_time = std::time::SystemTime::now(); let spawn_result = pool.spawn( ( procspawn::serde::Json(cfi_caches), - minidump.clone(), + minidump_path, spawn_time, ), - |(cfi_caches, minidump, spawn_time)| -> Result<_, ProcessMinidumpError> { + |(cfi_caches, minidump_path, spawn_time)| -> Result<_, ProcessMinidumpError> { let procspawn::serde::Json(cfi_caches) = cfi_caches; if let Ok(duration) = spawn_time.elapsed() { @@ -1831,7 +1793,11 @@ impl SymbolicationActor { // Stackwalk the minidump. let cfi = load_cfi_for_processor(cfi_caches); - let minidump = ByteView::from_slice(&minidump); + // we cannot map an `io::Error` into `MinidumpNotFound` since there is no public + // constructor on `ProcessResult`. Passing in an empty buffer should result in + // the same error though. + let minidump = + ByteView::open(minidump_path).unwrap_or_else(|_| ByteView::from_slice(b"")); let process_state = ProcessState::from_minidump(&minidump, Some(&cfi))?; let minidump_state = MinidumpState::new(&process_state); let object_type = minidump_state.object_type(); @@ -1910,8 +1876,6 @@ impl SymbolicationActor { spawn_result, Duration::from_secs(60), "minidump.stackwalk.spawn.error", - &minidump, - diagnostics_cache, ) }; @@ -1939,12 +1903,49 @@ impl SymbolicationActor { let mut iterations = 0; + let mut minidump_file = tempfile::Builder::new(); + minidump_file.prefix("minidump").suffix(".dmp"); + let minidump_file = if let Some(dir) = self.diagnostics_cache.cache_dir() { + std::fs::create_dir_all(dir)?; + minidump_file.tempfile_in(dir) + } else { + minidump_file.tempfile() + }?; + + minidump_file.as_file().write_all(&*minidump)?; + let result = loop { iterations += 1; - let result = self - .stackwalk_minidump_with_cfi(minidump.clone(), &cfi_caches) - .await?; + let result = match self + .stackwalk_minidump_with_cfi(&minidump_file, &cfi_caches) + .await + { + Ok(res) => res, + Err(err) => { + if self.diagnostics_cache.cache_dir().is_some() { + match minidump_file.keep() { + Ok((_file, path)) => { + sentry::configure_scope(|scope| { + scope.set_extra( + "crashed_minidump", + sentry::protocol::Value::String( + path.to_string_lossy().to_string(), + ), + ); + }); + } + Err(e) => log::error!("Failed to save minidump {:?}", &e), + }; + } else { + log::debug!("No diagnostics retention configured, not saving minidump"); + } + + // we explicitly match and return here, otherwise the borrow checker will + // complain that `minidump_file` is being moved into `save_minidump` + return Err(err); + } + }; let missing_modules: Vec<(CodeModuleId, &RawObjectInfo)> = result .referenced_modules From d20c247edaa377f60c0646da6702b6f790a05ad4 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 30 Jul 2021 11:12:15 +0200 Subject: [PATCH 2/6] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16768a4e9..60e1b4d53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ ### Internal - Measure how long it takes to download sources from external servers. ([#483](https://github.com/getsentry/symbolicator/pull/483)) +- Avoid serialization overhead when processing large minidumps out-of-process. ([#508](https://github.com/getsentry/symbolicator/pull/508)) ## 0.3.4 From bc0c3543e162451cfc57e6ac759dc6ebd3a1925c Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 30 Jul 2021 14:03:29 +0200 Subject: [PATCH 3/6] switch to write the file directly --- Cargo.lock | 4 - crates/symbolicator/Cargo.toml | 1 - crates/symbolicator/src/endpoints/minidump.rs | 29 +++++-- .../src/services/symbolication.rs | 83 +++++++++---------- 4 files changed, 63 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ce04d108..2ec5210e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -384,9 +384,6 @@ name = "bytes" version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b700ce4376041dcd0a327fd0097c41095743c4c8af8887265942faf1100bd040" -dependencies = [ - "serde", -] [[package]] name = "bzip2" @@ -3619,7 +3616,6 @@ dependencies = [ "apple-crash-report-parser", "backtrace", "base64 0.13.0", - "bytes 1.0.1", "cadence", "chrono", "console", diff --git a/crates/symbolicator/Cargo.toml b/crates/symbolicator/Cargo.toml index a9d19b971..f86bef0b8 100644 --- a/crates/symbolicator/Cargo.toml +++ b/crates/symbolicator/Cargo.toml @@ -11,7 +11,6 @@ anyhow = "1.0.38" apple-crash-report-parser = "0.4.2" backtrace = "0.3.55" base64 = "0.13.0" -bytes = { version = "1.0.1", features = ["serde"] } cadence = "0.25.0" chrono = { version = "0.4.19", features = ["serde"] } console = "0.14.0" diff --git a/crates/symbolicator/src/endpoints/minidump.rs b/crates/symbolicator/src/endpoints/minidump.rs index 944357edf..c5c9311a3 100644 --- a/crates/symbolicator/src/endpoints/minidump.rs +++ b/crates/symbolicator/src/endpoints/minidump.rs @@ -1,12 +1,12 @@ +use std::io::Write; + use actix_web::{error, multipart, App, Error, HttpMessage, HttpRequest, Json, Query, State}; use futures::{compat::Stream01CompatExt, StreamExt}; use crate::endpoints::symbolicate::SymbolicationRequestQueryParams; use crate::services::Service; use crate::types::{RequestOptions, SymbolicationResponse}; -use crate::utils::multipart::{ - read_multipart_file, read_multipart_request_options, read_multipart_sources, -}; +use crate::utils::multipart::{read_multipart_request_options, read_multipart_sources}; use crate::utils::sentry::ConfigureScope; async fn handle_minidump_request( @@ -32,17 +32,34 @@ async fn handle_minidump_request( let content_disposition = field.content_disposition(); match content_disposition.as_ref().and_then(|d| d.get_name()) { - Some("upload_file_minidump") => minidump = Some(read_multipart_file(field).await?), + Some("upload_file_minidump") => { + let minidump_file = tempfile::Builder::new() + .prefix("minidump") + .suffix(".dmp") + .tempfile()?; + let (mut file, temp_path) = minidump_file.into_parts(); + + let mut stream = field.compat(); + // it would have been nice to use `tokio::io::copy`, or at least the non-blocking + // `tokio::fs::File` / `tokio::io::AsyncWriteExt`, but that needs to run in the + // context of a tokio 1 executor, which these futures do not. + //let mut file = tokio::fs::File::from_std(file); + while let Some(chunk) = stream.next().await { + file.write_all(&chunk?)?; + } + + minidump = Some(temp_path) + } Some("sources") => sources = read_multipart_sources(field).await?.into(), Some("options") => options = read_multipart_request_options(field).await?, _ => (), // Always ignore unknown fields. } } - let minidump = minidump.ok_or_else(|| error::ErrorBadRequest("missing minidump"))?; + let minidump_file = minidump.ok_or_else(|| error::ErrorBadRequest("missing minidump"))?; let symbolication = state.symbolication(); - let request_id = symbolication.process_minidump(params.scope, minidump, sources, options); + let request_id = symbolication.process_minidump(params.scope, minidump_file, sources, options); match symbolication.get_response(request_id, params.timeout).await { Some(response) => Ok(Json(response)), diff --git a/crates/symbolicator/src/services/symbolication.rs b/crates/symbolicator/src/services/symbolication.rs index 7d97d7ff0..cb7649068 100644 --- a/crates/symbolicator/src/services/symbolication.rs +++ b/crates/symbolicator/src/services/symbolication.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::convert::TryInto; use std::future::Future; -use std::io::{Cursor, Write}; +use std::io::Cursor; use std::iter::FromIterator; use std::path::PathBuf; use std::sync::Arc; @@ -9,7 +9,6 @@ use std::time::{Duration, Instant}; use anyhow::Context; use apple_crash_report_parser::AppleCrashReport; -use bytes::Bytes; use chrono::{DateTime, TimeZone, Utc}; use futures::{channel::oneshot, future, FutureExt as _}; use parking_lot::Mutex; @@ -27,7 +26,7 @@ use symbolic::minidump::cfi::CfiCache; use symbolic::minidump::processor::{ CodeModule, CodeModuleId, FrameTrust, ProcessMinidumpError, ProcessState, RegVal, }; -use tempfile::NamedTempFile; +use tempfile::TempPath; use thiserror::Error; use crate::cache::CacheStatus; @@ -1770,12 +1769,12 @@ impl SymbolicationActor { /// modules. async fn stackwalk_minidump_with_cfi( &self, - minidump_file: &NamedTempFile, + minidump_file: &TempPath, cfi_caches: &CfiCacheModules, ) -> anyhow::Result { let pool = self.spawnpool.clone(); let cfi_caches = cfi_caches.for_processing(); - let minidump_path = minidump_file.path().to_owned(); + let minidump_path = minidump_file.to_path_buf(); let lazy = async move { let spawn_time = std::time::SystemTime::now(); let spawn_result = pool.spawn( @@ -1889,31 +1888,19 @@ impl SymbolicationActor { async fn do_stackwalk_minidump( self, scope: Scope, - minidump: Vec, + minidump_file: TempPath, sources: Arc<[SourceConfig]>, options: RequestOptions, ) -> Result<(SymbolicateStacktraces, MinidumpState), SymbolicationError> { let future = async move { - let minidump = Bytes::from(minidump); - - log::debug!("Processing minidump ({} bytes)", minidump.len()); - metric!(time_raw("minidump.upload.size") = minidump.len() as u64); + let len = minidump_file.metadata()?.len(); + log::debug!("Processing minidump ({} bytes)", len); + metric!(time_raw("minidump.upload.size") = len); let mut cfi_caches = CfiCacheModules::new(); let mut iterations = 0; - let mut minidump_file = tempfile::Builder::new(); - minidump_file.prefix("minidump").suffix(".dmp"); - let minidump_file = if let Some(dir) = self.diagnostics_cache.cache_dir() { - std::fs::create_dir_all(dir)?; - minidump_file.tempfile_in(dir) - } else { - minidump_file.tempfile() - }?; - - minidump_file.as_file().write_all(&*minidump)?; - let result = loop { iterations += 1; @@ -1923,20 +1910,23 @@ impl SymbolicationActor { { Ok(res) => res, Err(err) => { - if self.diagnostics_cache.cache_dir().is_some() { - match minidump_file.keep() { - Ok((_file, path)) => { - sentry::configure_scope(|scope| { - scope.set_extra( - "crashed_minidump", - sentry::protocol::Value::String( - path.to_string_lossy().to_string(), - ), - ); - }); - } - Err(e) => log::error!("Failed to save minidump {:?}", &e), - }; + if let Some(dir) = self.diagnostics_cache.cache_dir() { + if let Some(file_name) = minidump_file.file_name() { + let path = dir.join(file_name); + match minidump_file.persist(&path) { + Ok(_) => { + sentry::configure_scope(|scope| { + scope.set_extra( + "crashed_minidump", + sentry::protocol::Value::String( + path.to_string_lossy().to_string(), + ), + ); + }); + } + Err(e) => log::error!("Failed to save minidump {:?}", &e), + }; + } } else { log::debug!("No diagnostics retention configured, not saving minidump"); } @@ -2002,13 +1992,13 @@ impl SymbolicationActor { async fn do_process_minidump( self, scope: Scope, - minidump: Vec, + minidump_file: TempPath, sources: Arc<[SourceConfig]>, options: RequestOptions, ) -> Result { let (request, state) = self .clone() - .do_stackwalk_minidump(scope, minidump, sources, options) + .do_stackwalk_minidump(scope, minidump_file, sources, options) .await?; let mut response = self.do_symbolicate(request).await?; @@ -2020,14 +2010,16 @@ impl SymbolicationActor { pub fn process_minidump( &self, scope: Scope, - minidump: Vec, + minidump_file: TempPath, sources: Arc<[SourceConfig]>, options: RequestOptions, ) -> RequestId { - self.create_symbolication_request( - self.clone() - .do_process_minidump(scope, minidump, sources, options), - ) + self.create_symbolication_request(self.clone().do_process_minidump( + scope, + minidump_file, + sources, + options, + )) } } @@ -2283,9 +2275,12 @@ impl From<&SymCacheError> for ObjectFileStatus { #[cfg(test)] mod tests { + use tempfile::NamedTempFile; + use super::*; use std::fs; + use std::io::Write; use crate::config::Config; use crate::services::Service; @@ -2477,11 +2472,13 @@ mod tests { let (_symsrv, source) = test::symbol_server(); let minidump = test::read_fixture(path); + let mut minidump_file = NamedTempFile::new()?; + minidump_file.write_all(&minidump)?; let symbolication = service.symbolication(); let response = test::spawn_compat(move || async move { let request_id = symbolication.process_minidump( Scope::Global, - minidump, + minidump_file.into_temp_path(), Arc::new([source]), RequestOptions { dif_candidates: true, From 748bf4a7f72d70fb8de012a3b9771cf3f8af94be Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 30 Jul 2021 14:40:29 +0200 Subject: [PATCH 4/6] use a temp file for apple crash reports as well --- .../src/endpoints/applecrashreport.rs | 6 +++++- crates/symbolicator/src/endpoints/minidump.rs | 15 ++++----------- .../symbolicator/src/services/symbolication.rs | 12 ++++++------ crates/symbolicator/src/utils/multipart.rs | 18 +++++++++++++----- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/crates/symbolicator/src/endpoints/applecrashreport.rs b/crates/symbolicator/src/endpoints/applecrashreport.rs index a664c48bb..d71546da3 100644 --- a/crates/symbolicator/src/endpoints/applecrashreport.rs +++ b/crates/symbolicator/src/endpoints/applecrashreport.rs @@ -32,7 +32,11 @@ async fn handle_apple_crash_report_request( let content_disposition = field.content_disposition(); match content_disposition.as_ref().and_then(|d| d.get_name()) { - Some("apple_crash_report") => report = Some(read_multipart_file(field).await?), + Some("apple_crash_report") => { + let mut report_file = tempfile::tempfile()?; + read_multipart_file(field, &mut report_file).await?; + report = Some(report_file) + } Some("sources") => sources = read_multipart_sources(field).await?.into(), Some("options") => options = read_multipart_request_options(field).await?, _ => (), // Always ignore unknown fields. diff --git a/crates/symbolicator/src/endpoints/minidump.rs b/crates/symbolicator/src/endpoints/minidump.rs index c5c9311a3..ad56bb1ed 100644 --- a/crates/symbolicator/src/endpoints/minidump.rs +++ b/crates/symbolicator/src/endpoints/minidump.rs @@ -1,12 +1,12 @@ -use std::io::Write; - use actix_web::{error, multipart, App, Error, HttpMessage, HttpRequest, Json, Query, State}; use futures::{compat::Stream01CompatExt, StreamExt}; use crate::endpoints::symbolicate::SymbolicationRequestQueryParams; use crate::services::Service; use crate::types::{RequestOptions, SymbolicationResponse}; -use crate::utils::multipart::{read_multipart_request_options, read_multipart_sources}; +use crate::utils::multipart::{ + read_multipart_file, read_multipart_request_options, read_multipart_sources, +}; use crate::utils::sentry::ConfigureScope; async fn handle_minidump_request( @@ -39,14 +39,7 @@ async fn handle_minidump_request( .tempfile()?; let (mut file, temp_path) = minidump_file.into_parts(); - let mut stream = field.compat(); - // it would have been nice to use `tokio::io::copy`, or at least the non-blocking - // `tokio::fs::File` / `tokio::io::AsyncWriteExt`, but that needs to run in the - // context of a tokio 1 executor, which these futures do not. - //let mut file = tokio::fs::File::from_std(file); - while let Some(chunk) = stream.next().await { - file.write_all(&chunk?)?; - } + read_multipart_file(field, &mut file).await?; minidump = Some(temp_path) } diff --git a/crates/symbolicator/src/services/symbolication.rs b/crates/symbolicator/src/services/symbolication.rs index cb7649068..a1942009a 100644 --- a/crates/symbolicator/src/services/symbolication.rs +++ b/crates/symbolicator/src/services/symbolication.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::convert::TryInto; +use std::fs::File; use std::future::Future; -use std::io::Cursor; use std::iter::FromIterator; use std::path::PathBuf; use std::sync::Arc; @@ -2074,12 +2074,12 @@ impl SymbolicationActor { async fn parse_apple_crash_report( &self, scope: Scope, - minidump: Vec, + report: File, sources: Arc<[SourceConfig]>, options: RequestOptions, ) -> Result<(SymbolicateStacktraces, AppleCrashReportState), SymbolicationError> { let parse_future = async { - let report = AppleCrashReport::from_reader(Cursor::new(minidump))?; + let report = AppleCrashReport::from_reader(report)?; let mut metadata = report.metadata; let arch = report @@ -2188,7 +2188,7 @@ impl SymbolicationActor { async fn do_process_apple_crash_report( self, scope: Scope, - report: Vec, + report: File, sources: Arc<[SourceConfig]>, options: RequestOptions, ) -> Result { @@ -2204,7 +2204,7 @@ impl SymbolicationActor { pub fn process_apple_crash_report( &self, scope: Scope, - apple_crash_report: Vec, + apple_crash_report: File, sources: Arc<[SourceConfig]>, options: RequestOptions, ) -> RequestId { @@ -2520,7 +2520,7 @@ mod tests { let (service, _cache_dir) = setup_service(); let (_symsrv, source) = test::symbol_server(); - let report_file = test::read_fixture("apple_crash_report.txt"); + let report_file = std::fs::File::open("apple_crash_report.txt")?; let response = test::spawn_compat(move || async move { let request_id = service.symbolication().process_apple_crash_report( Scope::Global, diff --git a/crates/symbolicator/src/utils/multipart.rs b/crates/symbolicator/src/utils/multipart.rs index eefe828a2..a614329b2 100644 --- a/crates/symbolicator/src/utils/multipart.rs +++ b/crates/symbolicator/src/utils/multipart.rs @@ -1,3 +1,6 @@ +use std::fs::File; +use std::io::Write; + use actix_web::{dev::Payload, error, multipart, Error}; use futures::{compat::Stream01CompatExt, StreamExt}; @@ -26,15 +29,20 @@ pub async fn read_multipart_data( Ok(body) } -pub async fn read_multipart_file(field: multipart::Field) -> Result, Error> { - let mut body = Vec::with_capacity(512); +pub async fn read_multipart_file( + field: multipart::Field, + file: &mut File, +) -> Result<(), Error> { let mut stream = field.compat(); - + // it would have been nice to use `tokio::io::copy`, or at least the non-blocking + // `tokio::fs::File` / `tokio::io::AsyncWriteExt`, but that needs to run in the + // context of a tokio 1 executor, which these futures do not. + //let mut file = tokio::fs::File::from_std(file); while let Some(chunk) = stream.next().await { - body.extend_from_slice(&chunk?); + file.write_all(&chunk?)?; } - Ok(body) + Ok(()) } pub async fn read_multipart_sources( From ba3ba9214395269c70107d63ab3e89524e61b30e Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 30 Jul 2021 14:59:15 +0200 Subject: [PATCH 5/6] fix fixture, and seek to the beginning after writing --- crates/symbolicator/src/services/symbolication.rs | 4 ++-- crates/symbolicator/src/utils/multipart.rs | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/symbolicator/src/services/symbolication.rs b/crates/symbolicator/src/services/symbolication.rs index a1942009a..9ef5fab59 100644 --- a/crates/symbolicator/src/services/symbolication.rs +++ b/crates/symbolicator/src/services/symbolication.rs @@ -2284,7 +2284,7 @@ mod tests { use crate::config::Config; use crate::services::Service; - use crate::test; + use crate::test::{self, fixture}; /// Setup tests and create a test service. /// @@ -2520,7 +2520,7 @@ mod tests { let (service, _cache_dir) = setup_service(); let (_symsrv, source) = test::symbol_server(); - let report_file = std::fs::File::open("apple_crash_report.txt")?; + let report_file = std::fs::File::open(fixture("apple_crash_report.txt"))?; let response = test::spawn_compat(move || async move { let request_id = service.symbolication().process_apple_crash_report( Scope::Global, diff --git a/crates/symbolicator/src/utils/multipart.rs b/crates/symbolicator/src/utils/multipart.rs index a614329b2..ec74c3022 100644 --- a/crates/symbolicator/src/utils/multipart.rs +++ b/crates/symbolicator/src/utils/multipart.rs @@ -1,5 +1,5 @@ use std::fs::File; -use std::io::Write; +use std::io::{Seek, SeekFrom, Write}; use actix_web::{dev::Payload, error, multipart, Error}; use futures::{compat::Stream01CompatExt, StreamExt}; @@ -41,6 +41,7 @@ pub async fn read_multipart_file( while let Some(chunk) = stream.next().await { file.write_all(&chunk?)?; } + file.seek(SeekFrom::Start(0))?; Ok(()) } From b773a653166b4e8ed76d9e9ca0d1b7edbfe96a27 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 2 Aug 2021 11:42:20 +0200 Subject: [PATCH 6/6] use the tmp cache dir for minidump requests --- crates/symbolicator/src/endpoints/minidump.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/symbolicator/src/endpoints/minidump.rs b/crates/symbolicator/src/endpoints/minidump.rs index ad56bb1ed..ed730a749 100644 --- a/crates/symbolicator/src/endpoints/minidump.rs +++ b/crates/symbolicator/src/endpoints/minidump.rs @@ -33,10 +33,13 @@ async fn handle_minidump_request( let content_disposition = field.content_disposition(); match content_disposition.as_ref().and_then(|d| d.get_name()) { Some("upload_file_minidump") => { - let minidump_file = tempfile::Builder::new() - .prefix("minidump") - .suffix(".dmp") - .tempfile()?; + let mut minidump_file = tempfile::Builder::new(); + minidump_file.prefix("minidump").suffix(".dmp"); + let minidump_file = if let Some(tmp_dir) = state.config().cache_dir("tmp") { + minidump_file.tempfile_in(tmp_dir) + } else { + minidump_file.tempfile() + }?; let (mut file, temp_path) = minidump_file.into_parts(); read_multipart_file(field, &mut file).await?;