Skip to content

Commit 0b7b0f5

Browse files
authored
ref: Make SourceBundleDebugSession: Send, Sync and AsSelf (#767)
This replaces all usages of `LazyCell` (which is not thread safe) with `OnceCell`, which is and will also be available as `std::sync::OnceLock` in the future.
1 parent 9c06f36 commit 0b7b0f5

File tree

5 files changed

+36
-16
lines changed

5 files changed

+36
-16
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
**Features**:
6+
7+
- Replace internal usage of `LazyCell` by `OnceCell` and make `SourceBundleDebugSession`: `Send`, `Sync` and `AsSelf`. ([#767](https://github.com/getsentry/symbolic/pull/767))
8+
39
## 12.0.0
410

511
**Features**:

symbolic-debuginfo/Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ default = ["breakpad", "elf", "macho", "ms", "ppdb", "sourcebundle", "js", "wasm
2525
# Breakpad text format parsing and processing
2626
breakpad = ["nom", "nom-supreme", "regex"]
2727
# DWARF processing.
28-
dwarf = ["gimli", "lazycell"]
28+
dwarf = ["gimli", "once_cell"]
2929
# ELF reading
3030
elf = [
3131
"dwarf",
@@ -53,7 +53,7 @@ ms = [
5353
"goblin/pe32",
5454
"goblin/pe64",
5555
"goblin/std",
56-
"lazycell",
56+
"once_cell",
5757
"parking_lot",
5858
"pdb-addr2line",
5959
"scroll",
@@ -65,7 +65,7 @@ ppdb = [
6565
# Source bundle creation
6666
sourcebundle = [
6767
"lazy_static",
68-
"lazycell",
68+
"once_cell",
6969
"parking_lot",
7070
"regex",
7171
"serde_json",
@@ -94,7 +94,7 @@ gimli = { version = "0.27.0", optional = true, default-features = false, feature
9494
] }
9595
goblin = { version = "0.6.0", optional = true, default-features = false }
9696
lazy_static = { version = "1.4.0", optional = true }
97-
lazycell = { version = "1.2.1", optional = true }
97+
once_cell = { version = "1.17.1", optional = true }
9898
nom = { version = "7.0.0", optional = true }
9999
nom-supreme = { version = "0.8.0", optional = true }
100100
parking_lot = { version = "0.12.0", optional = true }

symbolic-debuginfo/src/dwarf.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use std::sync::Arc;
2020
use fallible_iterator::FallibleIterator;
2121
use gimli::read::{AttributeValue, Error as GimliError, Range};
2222
use gimli::{constants, DwarfFileType, UnitSectionOffset};
23-
use lazycell::LazyCell;
23+
use once_cell::sync::OnceCell;
2424
use thiserror::Error;
2525

2626
use symbolic_common::{AsSelf, Language, Name, NameMangling, SelfCell};
@@ -1109,7 +1109,7 @@ impl<'data> DwarfSections<'data> {
11091109
struct DwarfInfo<'data> {
11101110
inner: DwarfInner<'data>,
11111111
headers: Vec<UnitHeader<'data>>,
1112-
units: Vec<LazyCell<Option<Unit<'data>>>>,
1112+
units: Vec<OnceCell<Option<Unit<'data>>>>,
11131113
symbol_map: SymbolMap<'data>,
11141114
address_offset: i64,
11151115
kind: ObjectKind,
@@ -1153,7 +1153,7 @@ impl<'d> DwarfInfo<'d> {
11531153

11541154
// Prepare random access to unit headers.
11551155
let headers = inner.units().collect::<Vec<_>>()?;
1156-
let units = headers.iter().map(|_| LazyCell::new()).collect();
1156+
let units = headers.iter().map(|_| OnceCell::new()).collect();
11571157

11581158
Ok(DwarfInfo {
11591159
inner,
@@ -1173,7 +1173,7 @@ impl<'d> DwarfInfo<'d> {
11731173
None => return Ok(None),
11741174
};
11751175

1176-
let unit_opt = cell.try_borrow_with(|| {
1176+
let unit_opt = cell.get_or_try_init(|| {
11771177
// Parse the compilation unit from the header. This requires a top-level DIE that
11781178
// describes the unit itself. For some older DWARF files, this DIE might be missing
11791179
// which causes gimli to error out. We prefer to skip them silently as this simply marks

symbolic-debuginfo/src/ppdb.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::collections::HashMap;
44
use std::fmt;
55
use std::iter;
66

7-
use lazycell::LazyCell;
7+
use once_cell::sync::OnceCell;
88
use symbolic_common::{Arch, CodeId, DebugId};
99
use symbolic_ppdb::EmbeddedSource;
1010
use symbolic_ppdb::{Document, FormatError, PortablePdb};
@@ -143,7 +143,7 @@ impl fmt::Debug for PortablePdbObject<'_> {
143143
/// A debug session for a Portable PDB object.
144144
pub struct PortablePdbDebugSession<'data> {
145145
ppdb: PortablePdb<'data>,
146-
sources: LazyCell<HashMap<String, PPDBSource<'data>>>,
146+
sources: OnceCell<HashMap<String, PPDBSource<'data>>>,
147147
}
148148

149149
#[derive(Debug, Clone)]
@@ -156,7 +156,7 @@ impl<'data> PortablePdbDebugSession<'data> {
156156
fn new(ppdb: &'_ PortablePdb<'data>) -> Result<Self, FormatError> {
157157
Ok(PortablePdbDebugSession {
158158
ppdb: ppdb.clone(),
159-
sources: LazyCell::new(),
159+
sources: OnceCell::new(),
160160
})
161161
}
162162

@@ -196,7 +196,7 @@ impl<'data> PortablePdbDebugSession<'data> {
196196
&self,
197197
path: &str,
198198
) -> Result<Option<SourceFileDescriptor<'_>>, FormatError> {
199-
let sources = self.sources.borrow_with(|| self.init_sources());
199+
let sources = self.sources.get_or_init(|| self.init_sources());
200200
match sources.get(path) {
201201
None => Ok(None),
202202
Some(PPDBSource::Embedded(source)) => source.get_contents().map(|bytes| {

symbolic-debuginfo/src/sourcebundle.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use std::io::{BufReader, BufWriter, Read, Seek, Write};
5151
use std::path::Path;
5252
use std::sync::Arc;
5353

54-
use lazycell::LazyCell;
54+
use once_cell::sync::OnceCell;
5555
use parking_lot::Mutex;
5656
use regex::Regex;
5757
use serde::{Deserialize, Deserializer, Serialize};
@@ -676,7 +676,7 @@ impl<'data> SourceBundle<'data> {
676676
Ok(SourceBundleDebugSession {
677677
manifest: self.manifest.clone(),
678678
archive: self.archive.clone(),
679-
indexed_files: LazyCell::new(),
679+
indexed_files: OnceCell::new(),
680680
})
681681
}
682682

@@ -802,7 +802,7 @@ enum FileKey<'a> {
802802
pub struct SourceBundleDebugSession<'data> {
803803
manifest: Arc<SourceBundleManifest>,
804804
archive: Arc<Mutex<zip::read::ZipArchive<std::io::Cursor<&'data [u8]>>>>,
805-
indexed_files: LazyCell<HashMap<FileKey<'data>, Arc<String>>>,
805+
indexed_files: OnceCell<HashMap<FileKey<'data>, Arc<String>>>,
806806
}
807807

808808
impl<'data> SourceBundleDebugSession<'data> {
@@ -820,7 +820,7 @@ impl<'data> SourceBundleDebugSession<'data> {
820820

821821
/// Get the indexed file mapping.
822822
fn indexed_files(&self) -> &HashMap<FileKey, Arc<String>> {
823-
self.indexed_files.borrow_with(|| {
823+
self.indexed_files.get_or_init(|| {
824824
let files = &self.manifest.files;
825825
let mut rv = HashMap::with_capacity(files.len());
826826

@@ -928,6 +928,14 @@ impl<'data, 'session> DebugSession<'session> for SourceBundleDebugSession<'data>
928928
}
929929
}
930930

931+
impl<'slf, 'data: 'slf> AsSelf<'slf> for SourceBundleDebugSession<'data> {
932+
type Ref = SourceBundleDebugSession<'slf>;
933+
934+
fn as_self(&'slf self) -> &Self::Ref {
935+
unsafe { std::mem::transmute(self) }
936+
}
937+
}
938+
931939
/// An iterator over source files in a SourceBundle object.
932940
pub struct SourceBundleFileIterator<'s> {
933941
files: std::collections::btree_map::Values<'s, String, SourceFileInfo>,
@@ -1362,6 +1370,12 @@ mod tests {
13621370
Ok(())
13631371
}
13641372

1373+
#[test]
1374+
fn debugsession_is_sendsync() {
1375+
fn is_sendsync<T: Send + Sync>() {}
1376+
is_sendsync::<SourceBundleDebugSession>();
1377+
}
1378+
13651379
#[test]
13661380
fn test_source_descriptor() -> Result<(), SourceBundleError> {
13671381
let mut writer = Cursor::new(Vec::new());

0 commit comments

Comments
 (0)