From 0c5433628b5ebf2e1b564ec7b2d3392928c22dae Mon Sep 17 00:00:00 2001 From: C J Silverio Date: Mon, 23 Jan 2023 16:49:42 -0800 Subject: [PATCH] Fixes issue #34: set tmpfile length in async writer The async `poll_write()` implementation was creating a tempfile as a backing for its inner mmap, but it was failing to set the length on the file to match the incoming data. Compare with the sync implementation! This bug was exposed when the `memmap2` crate was swapped in for `memmap`. The older crate was likely more lax about this. Wrote a pair of new tests for `cacache::write_hash_sync` and `cacache::write_hash`. The async test fails without this change, as does any benchmarks run. Everything passes with it. --- src/content/write.rs | 4 +++- src/put.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/content/write.rs b/src/content/write.rs index 677bb2b..4471c37 100644 --- a/src/content/write.rs +++ b/src/content/write.rs @@ -10,6 +10,7 @@ use async_std::task::{self, Context, JoinHandle, Poll}; use futures::io::AsyncWrite; use futures::prelude::*; use memmap2::MmapMut; +// use memmap::MmapMut; use ssri::{Algorithm, Integrity, IntegrityOpts}; use tempfile::NamedTempFile; @@ -121,11 +122,12 @@ impl AsyncWriter { .create(&tmp_path) .await .to_internal()?; - let tmpfile = task::spawn_blocking(|| NamedTempFile::new_in(tmp_path)) + let mut tmpfile = task::spawn_blocking(|| NamedTempFile::new_in(tmp_path)) .await .to_internal()?; let mmap = if let Some(size) = size { if size <= MAX_MMAP_SIZE { + tmpfile.as_file_mut().set_len(size as u64).to_internal()?; unsafe { MmapMut::map_mut(tmpfile.as_file()).ok() } } else { None diff --git a/src/put.rs b/src/put.rs index ec5ec2d..8670a9b 100644 --- a/src/put.rs +++ b/src/put.rs @@ -440,4 +440,34 @@ mod tests { let data = crate::read_sync(&dir, "hello").unwrap(); assert_eq!(data, b"hello"); } + + #[test] + fn hash_write_sync() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().to_owned(); + let original = format!("hello world{}", 5); + let integrity = crate::write_hash_sync(&dir, &original) + .expect("should be able to write a hash synchronously"); + let bytes = crate::read_hash_sync(&dir, &integrity) + .expect("should be able to read the data we just wrote"); + let result = + String::from_utf8(bytes).expect("we wrote valid utf8 but did not read valid utf8 back"); + assert_eq!(result, original, "we did not read back what we wrote"); + } + + #[async_attributes::test] + async fn hash_write_async() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().to_owned(); + let original = format!("hello world{}", 12); + let integrity = crate::write_hash(&dir, &original) + .await + .expect("should be able to write a hash asynchronously"); + let bytes = crate::read_hash(&dir, &integrity) + .await + .expect("should be able to read back what we wrote"); + let result = + String::from_utf8(bytes).expect("we wrote valid utf8 but did not read valid utf8 back"); + assert_eq!(result, original, "we did not read back what we wrote"); + } }