diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index f85b7b9191..6cf1df9b1e 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -103,8 +103,9 @@ impl Document { fn note_unreadable(&self, num: u32, reason: impl FnOnce() -> String) { // A poisoned lock only means the object goes unlisted; the read itself still fails. let Ok(mut unreadable) = self.unreadable.lock() else { return }; - if unreadable.wants(num) { - unreadable.note(num, self.generation(num), &reason()); + let hidden = |n: u32| self.overlay.contains_key(&n); + if unreadable.wants(num, hidden) { + unreadable.note(num, self.generation(num), &reason(), hidden); } } diff --git a/crates/cos/src/document/state.rs b/crates/cos/src/document/state.rs index a6171479ca..3627280960 100644 --- a/crates/cos/src/document/state.rs +++ b/crates/cos/src/document/state.rs @@ -62,7 +62,9 @@ pub(super) fn objstm_member(num: i64, offset: i64, first: Option, len: us } /// Objects the reader could not read, each with the reason (see `Document::unreadable_objects`). -/// Bounded: past [`Unreadable::MAX`] objects only the fact that there were more is kept. +/// Bounded: a document records past [`Unreadable::MAX`] of the objects it shows only the fact that +/// there were more. Entries a clone has edited are hidden from it and do not count, so the whole +/// list holds at most `MAX` plus the objects edited in clones. #[derive(Debug, Default)] pub(super) struct Unreadable { pub(super) objects: BTreeMap, @@ -74,13 +76,15 @@ impl Unreadable { /// Characters of a reason kept: a parser's message can quote the bytes it stopped at. const REASON: usize = 160; - /// Whether `num` is to be recorded: not listed yet and there is room. A full list notes that - /// there were more, so a caller asks this before building the reason. - pub(super) fn wants(&mut self, num: u32) -> bool { + /// Whether `num` is to be recorded: not listed yet and there is room among the entries the + /// asking document still shows (`hidden` are the ones it has edited, as in [`Self::lines`]), + /// so a clone's edits never use up its own allowance. A full list notes that there were more, + /// so a caller asks this before building the reason. + pub(super) fn wants(&mut self, num: u32, hidden: impl Fn(u32) -> bool) -> bool { if self.objects.contains_key(&num) { return false; } - if self.objects.len() >= Self::MAX { + if self.objects.keys().filter(|n| !hidden(**n)).count() >= Self::MAX { self.more = true; return false; } @@ -88,21 +92,23 @@ impl Unreadable { } /// Record `num` once; the first reason given is kept. - pub(super) fn note(&mut self, num: u32, generation: u16, reason: &str) { - if self.wants(num) { + pub(super) fn note(&mut self, num: u32, generation: u16, reason: &str, hidden: impl Fn(u32) -> bool) { + if self.wants(num, hidden) { self.objects.insert(num, (generation, reason.chars().take(Self::REASON).collect())); } } /// One line per recorded object not `hidden`, then one saying there were more, if there were. pub(super) fn lines(&self, hidden: impl Fn(u32) -> bool) -> Vec { - let mut lines: Vec = self - .objects - .iter() - .filter(|(num, _)| !hidden(**num)) + // Clones share the list and each hides its own edits, so the list may hold more than + // `MAX` entries in all; each document still shows at most `MAX`. + let mut shown = self.objects.iter().filter(|(num, _)| !hidden(**num)); + let mut lines: Vec = shown + .by_ref() + .take(Self::MAX) .map(|(num, (generation, reason))| format!("object {num} {generation} could not be read: {reason}")) .collect(); - if self.more { + if self.more || shown.next().is_some() { lines.push(format!("more objects could not be read; only the first {} are listed", Self::MAX)); } lines diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 2f40b25b07..4dbc99bb4e 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1919,6 +1919,32 @@ fn the_unreadable_object_list_is_bounded() { ); } +/// The unreadable list is shared by clones, and each hides what it has edited. A clone that edited +/// every listed object still lists its own failures: hidden entries don't use up its allowance. +#[test] +fn objects_a_clone_edited_do_not_use_up_its_unreadable_allowance() { + let mut bodies = vec!["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>"]; + bodies.extend(std::iter::repeat_n("<< /A ) >>", 110)); + let doc = Document::open(Arc::new(build(&bodies, "/Root 1 0 R"))).unwrap(); + for num in 3..103 { + assert!(doc.try_get(num).is_err()); + } + assert_eq!(doc.unreadable_objects().len(), 100); + let mut edited = doc.clone(); + for num in 3..103 { + edited.set(ObjRef::new(num, 0), Object::Null); + } + for num in 103..113 { + assert!(edited.try_get(num).is_err()); + } + let lines = edited.unreadable_objects(); + assert_eq!(lines.len(), 10, "{lines:?}"); + assert!(lines[0].starts_with("object 103 0 could not be read"), "{}", lines[0]); + let original = doc.unreadable_objects(); + assert_eq!(original.len(), 101, "the original still shows at most 100, then says there were more"); + assert_eq!(original[100], "more objects could not be read; only the first 100 are listed"); +} + /// The cache keeps the sum of what its objects hold as they come and go, so neither reader has /// to measure the cache again on every read (#5873). #[test]