Skip to content

Commit 987ddb6

Browse files
committed
fix(sandbox): cut executable identity hashing cost
Reuse in-call digests and optimize sha2 in dev builds. Signed-off-by: Eric Curtin <eric.curtin@docker.com>
1 parent dfef088 commit 987ddb6

2 files changed

Lines changed: 55 additions & 22 deletions

File tree

‎Cargo.toml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,3 +182,8 @@ strip = true
182182
[profile.dev]
183183
# Faster compile times for dev builds
184184
debug = 1
185+
186+
# Executable identity hashes the (100+ MiB) test binary. Unoptimized SHA-256
187+
# is ~10x slower, which times out broker tests.
188+
[profile.dev.package.sha2]
189+
opt-level = 3

‎crates/openshell-binary-identity/src/lib.rs‎

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,23 @@ fn resolve_linux_process(
121121
paths
122122
});
123123

124-
let (executable_identity, pending_cache_entry) =
125-
resolve_open_executable(&snapshot, &mut executable, cache)?;
126-
let mut pending_cache_entries = pending_cache_entry.into_iter().collect::<Vec<_>>();
124+
// Digests computed during this call. They reach the shared cache only after
125+
// every snapshot validates, but later processes in the chain reuse them.
126+
let mut pending_cache_entries = HashMap::new();
127+
let executable_identity = resolve_open_executable(
128+
&snapshot,
129+
&mut executable,
130+
cache,
131+
&mut pending_cache_entries,
132+
)?;
127133
let mut ancestors = Vec::with_capacity(ancestor_processes.len());
128134
for (ancestor, executable) in &mut ancestor_processes {
129-
let (identity, pending_cache_entry) = resolve_open_executable(ancestor, executable, cache)?;
130-
ancestors.push(identity);
131-
pending_cache_entries.extend(pending_cache_entry);
135+
ancestors.push(resolve_open_executable(
136+
ancestor,
137+
executable,
138+
cache,
139+
&mut pending_cache_entries,
140+
)?);
132141
}
133142

134143
validate_process_snapshot(pid, &snapshot)?;
@@ -151,23 +160,21 @@ fn resolve_open_executable(
151160
snapshot: &ProcessSnapshot,
152161
executable: &mut std::fs::File,
153162
cache: &Mutex<HashMap<ExecutableCacheKey, Sha256Digest>>,
154-
) -> Result<
155-
(
156-
ExecutableIdentity,
157-
Option<(ExecutableCacheKey, Sha256Digest)>,
158-
),
159-
ResolveError,
160-
> {
163+
pending: &mut HashMap<ExecutableCacheKey, Sha256Digest>,
164+
) -> Result<ExecutableIdentity, ResolveError> {
161165
let key = snapshot.executable_cache_key();
162-
let cached_digest = cached_executable_digest(cache, key);
163-
let digest = cached_digest.map_or_else(|| hash_executable(snapshot.pid, executable), Ok)?;
164-
Ok((
165-
ExecutableIdentity {
166-
path: snapshot.binary_path.clone(),
167-
digest: Some(digest),
168-
},
169-
cached_digest.is_none().then_some((key, digest)),
170-
))
166+
let known = pending
167+
.get(&key)
168+
.copied()
169+
.or_else(|| cached_executable_digest(cache, key));
170+
let digest = known.map_or_else(|| hash_executable(snapshot.pid, executable), Ok)?;
171+
if known.is_none() {
172+
pending.insert(key, digest);
173+
}
174+
Ok(ExecutableIdentity {
175+
path: snapshot.binary_path.clone(),
176+
digest: Some(digest),
177+
})
171178
}
172179

173180
#[cfg(target_os = "linux")]
@@ -545,6 +552,27 @@ mod tests {
545552
assert!(parent.digest.is_some());
546553
}
547554

555+
#[test]
556+
fn executable_is_hashed_once_per_resolution() {
557+
let pid = std::process::id();
558+
let (snapshot, mut executable) = open_process_snapshot(pid).unwrap();
559+
let cache = Mutex::new(HashMap::new());
560+
let mut pending = HashMap::new();
561+
562+
let hashed =
563+
resolve_open_executable(&snapshot, &mut executable, &cache, &mut pending).unwrap();
564+
assert_eq!(pending.len(), 1);
565+
566+
// A sentinel proves a pending digest is reused instead of rehashed.
567+
let sentinel: Sha256Digest = "ab".repeat(32).parse().unwrap();
568+
assert_ne!(hashed.digest, Some(sentinel));
569+
pending.insert(snapshot.executable_cache_key(), sentinel);
570+
let reused =
571+
resolve_open_executable(&snapshot, &mut executable, &cache, &mut pending).unwrap();
572+
assert_eq!(reused.digest, Some(sentinel));
573+
assert!(cache.lock().unwrap().is_empty());
574+
}
575+
548576
#[test]
549577
fn process_tree_root_does_not_escape_into_host_ancestry() {
550578
let pid = std::process::id();

0 commit comments

Comments
 (0)