From 84d4b8cab9b8ee1d6d0c3fbed70ea3291c22a1b4 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Mon, 2 Nov 2026 16:21:45 -0500 Subject: [PATCH] fix(cos): a load that read one clone's edits is not shared with the others Clones share the object cache and the parsed object streams. A clone that freed or replaced an object stream then read a member of it put the result in those shared maps: a failure (which this branch now caches) or, as on main, the replacement stream and the member parsed from it. Another clone whose stream is intact then read that instead of the file. A load that reads an unsaved edit (an edited object stream, or an edited indirect /Length) is no longer cached, and an edited object stream is parsed for its own document only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/cos/src/document/objects.rs | 30 ++++++++++++++++++++++-------- crates/cos/src/document/state.rs | 11 +++++++++++ crates/cos/src/document/tests.rs | 21 +++++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index 2ca327de74..55a01e117c 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -1,5 +1,7 @@ use super::reader::find; -use super::state::{LENGTHS, LoadGuard, Loaded, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, loaded_bytes, objstm_member}; +use super::state::{ + LENGTHS, LoadGuard, Loaded, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, loaded_bytes, note_overlay_read, objstm_member, overlay_reads, +}; use super::xref::{Revision, XrefEntry}; use super::{ Arc, CosError, Cow, Dict, Document, Lexer, MAX_DECODED, MAX_JBIG2_GLOBALS, ObjRef, Object, Stream, is_whitespace, parse_indirect_shared, @@ -383,10 +385,12 @@ impl Document { } pub fn try_get(&self, num: u32) -> Result, CosError> { - match self.overlay.get(&num) { - Some(Slot::Set(_, o)) => return Ok(o.clone()), - Some(Slot::Freed(_)) => return Ok(Arc::new(Object::Null)), - None => {} + if let Some(slot) = self.overlay.get(&num) { + note_overlay_read(); + return Ok(match slot { + Slot::Set(_, o) => o.clone(), + Slot::Freed(_) => Arc::new(Object::Null), + }); } if let Some(loaded) = self.cache.lock().map_err(|_| CosError::Poisoned)?.get(&num) { return loaded.clone(); @@ -395,10 +399,12 @@ impl Document { // cycles such as `6 0 obj << /Length 6 0 R >>`. Bound the nesting on every path. let _guard = LoadGuard::enter(num).ok_or_else(|| CosError::Syntax { offset: 0, detail: format!("reference cycle while loading object {num}") })?; + let edits_read = overlay_reads(); let loaded = self.load(num, 0).map(Arc::new); // A failure is kept like an object, or every read of a damaged object searches the whole - // file again. A poisoned lock says nothing about the object. - if !matches!(loaded, Err(CosError::Poisoned)) { + // file again. A poisoned lock says nothing about the object. Clones share the cache, so + // a load that read this document's edits (an edited object stream or `/Length`) is not kept. + if overlay_reads() == edits_read && !matches!(loaded, Err(CosError::Poisoned)) { // The editing cache is bounded like the scan reader's: re-parsing a file-backed view // is cheap, and returned objects outlive eviction. self.cache.lock().map_err(|_| CosError::Poisoned)?.insert(num, loaded.clone()); @@ -540,6 +546,9 @@ impl Document { None | Some(XrefEntry::Free { .. }) => Ok(Object::Null), Some(XrefEntry::InFile { offset, .. }) => { let resolve = |r: ObjRef| { + if self.overlay.contains_key(&r.num) { + note_overlay_read(); + } if let Some(known) = LENGTHS.with(|m| m.borrow().get(&r.num).copied()) { return known; } @@ -570,7 +579,9 @@ impl Document { } pub(super) fn objstm(&self, num: u32) -> Result, CosError> { - if let Some(s) = self.objstms.lock().map_err(|_| CosError::Poisoned)?.get(&num) { + // An edited or freed stream is this document's alone: clones share the parsed streams. + let edited = self.overlay.contains_key(&num); + if !edited && let Some(s) = self.objstms.lock().map_err(|_| CosError::Poisoned)?.get(&num) { return Ok(s.clone()); } // A file entry that cannot place the stream (one that says it is itself compressed @@ -596,6 +607,9 @@ impl Document { index.push(objstm_member(onum, off, first, data.len())); } let stm = Arc::new(ObjStm { index, data: Arc::new(data) }); + if edited { + return Ok(stm); + } // Same bound as the object cache: decoded object streams can be megabytes each. // Insert first so the retained `num` is actually in the map. self.objstms.lock().map_err(|_| CosError::Poisoned)?.insert(num, stm.clone()); diff --git a/crates/cos/src/document/state.rs b/crates/cos/src/document/state.rs index 121a59c2e7..286973943e 100644 --- a/crates/cos/src/document/state.rs +++ b/crates/cos/src/document/state.rs @@ -8,6 +8,17 @@ thread_local! { /// retries a parse several ways, and each retry would resolve the same lengths again /// (exponential in the chain of streams whose lengths point at further streams). pub(super) static LENGTHS: std::cell::RefCell>> = std::cell::RefCell::new(HashMap::new()); + /// Unsaved edits read on this thread (`Document::try_get`, and a `/Length` in `Document::load`). + static OVERLAY_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Unsaved edits read so far on this thread: a load during which this changes depended on one. +pub(super) fn overlay_reads() -> u64 { + OVERLAY_READS.with(std::cell::Cell::get) +} + +pub(super) fn note_overlay_read() { + OVERLAY_READS.with(|c| c.set(c.get().wrapping_add(1))); } /// Marks an object as being loaded on this thread. Loading can re-enter (indirect stream diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 3cebe0136d..ee7fd28750 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -613,6 +613,27 @@ fn an_unreadable_object_is_searched_for_once() { assert_eq!(edited.try_get(3).as_deref(), Ok(&Object::Null)); } +/// Clones share the cache, and an edit is one clone's alone: a member of an object stream that +/// one clone freed or replaced still reads, in the others, from the file's stream. +#[test] +fn an_edited_object_stream_leaves_other_clones_members_alone() { + let file = Arc::new(object_stream_file("3 0 ", "<< /Hidden (yes) >>", 0)); + let doc = Document::open(file.clone()).unwrap(); + let mut freed = doc.clone(); + freed.free(ObjRef::new(4, 0)); + assert_eq!(freed.try_get(3), Err(CosError::MissingObject(4))); + assert_eq!(text_of(&doc, 3, b"Hidden").as_deref(), Some("yes"), "the free reached another clone"); + let doc = Document::open(file).unwrap(); + let mut replaced = doc.clone(); + let mut dict = Dict::new(); + dict.set(b"Type".to_vec(), Object::name("ObjStm")); + dict.set(b"N".to_vec(), Object::Int(1)); + dict.set(b"First".to_vec(), Object::Int(4)); + replaced.set(ObjRef::new(4, 0), crate::Stream::from_raw(dict, b"3 0 << /Hidden (no) >>".to_vec())); + assert_eq!(text_of(&replaced, 3, b"Hidden").as_deref(), Some("no")); + assert_eq!(text_of(&doc, 3, b"Hidden").as_deref(), Some("yes"), "the replacement reached another clone"); +} + /// A classic table with `/Size` rows and object 0's free entry first, its subsection numbered /// `1 n` instead of `0 n` (#2676): every row is one number off. fn misnumbered_table(start: u32) -> Vec {