From eb9aa885050acd132e66c716f00f934b40d81507 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 15 Nov 2021 11:50:28 +0100 Subject: [PATCH 1/4] feat(byteview): Add non-consuming mmap constructor This adds constructor to mmap a File into a ByteView without consuming the file object. --- symbolic-common/src/byteview.rs | 51 +++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/symbolic-common/src/byteview.rs b/symbolic-common/src/byteview.rs index 8ac47a434..1332e3234 100644 --- a/symbolic-common/src/byteview.rs +++ b/symbolic-common/src/byteview.rs @@ -118,6 +118,8 @@ impl<'a> ByteView<'a> { /// Constructs a `ByteView` from an open file handle by memory mapping the file. /// + /// See [`ByteView::mmap_file`] for a non-consuming version of this constructor. + /// /// # Example /// /// ``` @@ -131,7 +133,28 @@ impl<'a> ByteView<'a> { /// } /// ``` pub fn map_file(file: File) -> Result { - let backing = match unsafe { Mmap::map(&file) } { + Self::mmap_file(&file) + } + + /// Constructs a `ByteView` from an open file handle by memory mapping the file. + /// + /// The main difference with [`ByteView::map_file`] is that this takes the [`File`] by + /// reference rather than consuming it. + /// + /// # Example + /// + /// ``` + /// use std::io::Write; + /// use symbolic_common::ByteView; + /// + /// fn main() -> Result<(), std::io::Error> { + /// let mut file = tempfile::tempfile()?; + /// let view = ByteView::mmap_file(&file)?; + /// Ok(()) + /// } + /// ``` + pub fn mmap_file(file: &File) -> Result { + let backing = match unsafe { Mmap::map(file) } { Ok(mmap) => ByteViewBacking::Mmap(mmap), Err(err) => { // this is raised on empty mmaps which we want to ignore. The 1006 Windows error @@ -234,7 +257,7 @@ unsafe impl StableDeref for ByteView<'_> {} mod tests { use super::*; - use std::io::Write; + use std::io::{Read, Seek, Write}; use similar_asserts::assert_eq; use tempfile::NamedTempFile; @@ -260,4 +283,28 @@ mod tests { Ok(()) } + + #[test] + fn test_mmap_fd_reuse() -> Result<(), std::io::Error> { + let mut tmp = NamedTempFile::new()?; + tmp.write_all(b"1234")?; + + let view = ByteView::mmap_file(tmp.as_file())?; + + // This deletes the file on disk. + let path = tmp.path().to_path_buf(); + let mut file = tmp.into_file(); + assert!(!path.exists()); + + // Ensure we can still read from the the file after mmapping and deleting it on disk. + let mut buf = Vec::new(); + file.rewind()?; + file.read_to_end(&mut buf)?; + assert_eq!(buf, b"1234"); + + // Ensure the byteview can still read the file as well. + assert_eq!(&*view, b"1234"); + + Ok(()) + } } From f15df74be8cdebb8513f4f9126d6e6d09fc525f5 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 15 Nov 2021 12:08:23 +0100 Subject: [PATCH 2/4] Explicitly drop the file before using the byteview --- symbolic-common/src/byteview.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/symbolic-common/src/byteview.rs b/symbolic-common/src/byteview.rs index 1332e3234..c0e7be3aa 100644 --- a/symbolic-common/src/byteview.rs +++ b/symbolic-common/src/byteview.rs @@ -301,6 +301,7 @@ mod tests { file.rewind()?; file.read_to_end(&mut buf)?; assert_eq!(buf, b"1234"); + drop(file); // Ensure the byteview can still read the file as well. assert_eq!(&*view, b"1234"); From d8aac07a5e4816a14e177f5135dc452ff189fa33 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Thu, 18 Nov 2021 09:25:54 +0100 Subject: [PATCH 3/4] Rename --- symbolic-common/src/byteview.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/symbolic-common/src/byteview.rs b/symbolic-common/src/byteview.rs index c0e7be3aa..85fded2dc 100644 --- a/symbolic-common/src/byteview.rs +++ b/symbolic-common/src/byteview.rs @@ -118,7 +118,7 @@ impl<'a> ByteView<'a> { /// Constructs a `ByteView` from an open file handle by memory mapping the file. /// - /// See [`ByteView::mmap_file`] for a non-consuming version of this constructor. + /// See [`ByteView::map_file_ref`] for a non-consuming version of this constructor. /// /// # Example /// @@ -133,7 +133,7 @@ impl<'a> ByteView<'a> { /// } /// ``` pub fn map_file(file: File) -> Result { - Self::mmap_file(&file) + Self::map_file_ref(&file) } /// Constructs a `ByteView` from an open file handle by memory mapping the file. @@ -149,11 +149,11 @@ impl<'a> ByteView<'a> { /// /// fn main() -> Result<(), std::io::Error> { /// let mut file = tempfile::tempfile()?; - /// let view = ByteView::mmap_file(&file)?; + /// let view = ByteView::map_file_ref(&file)?; /// Ok(()) /// } /// ``` - pub fn mmap_file(file: &File) -> Result { + pub fn map_file_ref(file: &File) -> Result { let backing = match unsafe { Mmap::map(file) } { Ok(mmap) => ByteViewBacking::Mmap(mmap), Err(err) => { @@ -289,7 +289,7 @@ mod tests { let mut tmp = NamedTempFile::new()?; tmp.write_all(b"1234")?; - let view = ByteView::mmap_file(tmp.as_file())?; + let view = ByteView::map_file_ref(tmp.as_file())?; // This deletes the file on disk. let path = tmp.path().to_path_buf(); From 872f78ff62f0aa1f8def8132fcb844610f68f298 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Thu, 18 Nov 2021 09:29:49 +0100 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 881e84f42..51154ab92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Unreleased + +**Features**: + +- Add `ByteView::map_file_ref` constructor which does not consume the `File` passed to it. ([#448](https://github.com/getsentry/symbolic/pull/448)) + +**Fixes**: + +- Support Unreal Engine 5 crash reporter. ([#449](https://github.com/getsentry/symbolic/pull/449)) + ## 8.4.0 **Features**: