forked from 1b-agent/opencadstudio-upstream
fix: close the review findings on the last web wave
Full verification of the XREF/data/ARCHIVE/worker wave (three
reviewers, one forensic) found no blockers and no crossfire damage;
five majors and several minors are fixed here.
- ARCHIVE now packages XREFs: the bytes registry added by the same
commit is consulted (the in-code 'no xref store exists' comment was
false within the commit), and the drawing is packaged only after
the full save preparation — without it kernel solids shipped
without their ACIS data.
- PendingClose stores the stable tab id instead of a vector index:
the web save now completes asynchronously, and closing a lower tab
during the serialization window shifted indices — closing the WRONG
tab, or panicking on a stale one. All producers and consumers
resolve the id at use.
- The xref bytes registry is capped (6 entries, oldest evicted): every
open, drop, and pick registered full drawing bytes with no bound —
an OOM waiting to happen. Reopened recents register too, so any
entry path can serve as another drawing's xref.
- A re-picked data-link source with the same name but different bytes
no longer silently replaces the first (the browser hands back bare
names, so two same-named files collided and every reference read
the newer pick): the pick is refused and the user is told why.
- The unresolved-xref warning lists file names (what to drop on the
page), not internal block names; the ZIP writer refused the ZIP64
sentinel values (0xFFFF counts, 0xFFFFFFFF sizes/offsets) instead
of emitting archives that send strict readers hunting for a ZIP64
EOCD that is absent; docs note the worker-side saves.
Known follow-ups documented, not fixed here: the serde-skipped
ExtendedEntity/CadDocument raw bytes still drop on worker crossings
(needs codec accessors — upstream), and WBLOCK/SAVEALL/ARCHIVE
still serialize inline (the worker route is a small follow-up).
(cherry picked from commit 6f429f24)
Ported onto main (#1717 review): main already keys PendingClose by tab
id, so those hunks stay as on main; the ZIP64 guard is moot with the zip
crate; the unresolved-xref names split on both path separators, since
Path does not split Windows paths in the browser build.
This commit is contained in:
1 parent
61c32c7f74
commit
8968e79942
7 files changed
+115
-17
No files matched your search
@@ -71,11 +71,42 @@ fn same_source_name(a: &str, b: &str) -> bool {
|
||||
// Only the web registers sources (native re-reads the real file, so a file
|
||||
// edited after picking is picked up); tests exercise the registry directly.
|
||||
#[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))]
|
||||
pub(crate) fn register_picked_source(path: &str, bytes: Arc<Vec<u8>>) {
|
||||
picked_sources()
|
||||
/// Returns false when the registration REPLACED different bytes under the
|
||||
/// same key — the browser hands back bare file names, so two distinct files
|
||||
/// named alike collide and every reference to that name would silently read
|
||||
/// the newer pick. Callers surface that to the user.
|
||||
pub(crate) fn register_picked_source(path: &str, bytes: Arc<Vec<u8>>) -> bool {
|
||||
let mut cache = picked_sources()
|
||||
.lock()
|
||||
.unwrap_or_else(|error| error.into_inner());
|
||||
let replaced = cache
|
||||
.get(path)
|
||||
.is_some_and(|old| old.as_slice() != bytes.as_slice());
|
||||
if replaced {
|
||||
take_last_collision().replace(file_name_of(path));
|
||||
}
|
||||
cache.insert(path.to_string(), bytes);
|
||||
!replaced
|
||||
}
|
||||
|
||||
/// The name whose re-pick was refused because different bytes are already
|
||||
/// registered under it — the browse handler reports and clears it.
|
||||
fn last_collision() -> &'static Mutex<Option<String>> {
|
||||
static LAST: OnceLock<Mutex<Option<String>>> = OnceLock::new();
|
||||
LAST.get_or_init(|| Mutex::new(None))
|
||||
}
|
||||
|
||||
fn take_last_collision() -> std::sync::MutexGuard<'static, Option<String>> {
|
||||
last_collision()
|
||||
.lock()
|
||||
.unwrap_or_else(|error| error.into_inner())
|
||||
.insert(path.to_string(), bytes);
|
||||
}
|
||||
|
||||
fn file_name_of(path: &str) -> String {
|
||||
std::path::Path::new(path)
|
||||
.file_name()
|
||||
.map(|n| n.to_string_lossy().into_owned())
|
||||
.unwrap_or_else(|| path.to_string())
|
||||
}
|
||||
|
||||
/// Bytes registered for this reference — exact key first, then any entry that
|
||||
@@ -1283,7 +1314,15 @@ impl OpenCADStudio {
|
||||
// keep its bytes so preview, save, and later re-opens read
|
||||
// them instead of asking for the picker again.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
register_picked_source(&path.to_string_lossy(), Arc::new(handle.read().await));
|
||||
{
|
||||
let fresh = register_picked_source(
|
||||
&path.to_string_lossy(),
|
||||
Arc::new(handle.read().await),
|
||||
);
|
||||
if !fresh {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
Some(path)
|
||||
},
|
||||
Message::DataLinkBrowseResult,
|
||||
@@ -1291,6 +1330,15 @@ impl OpenCADStudio {
|
||||
}
|
||||
|
||||
pub(super) fn on_data_link_browse_result(&mut self, path: Option<PathBuf>) -> Task<Message> {
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
if let Some(name) = take_last_collision().take() {
|
||||
self.command_line.push_warning(
|
||||
crate::tf!(
|
||||
"\"{name}\" is already loaded from another folder with different contents; re-picking it was ignored to keep the existing link."
|
||||
)
|
||||
.as_ref(),
|
||||
);
|
||||
}
|
||||
if let Some(path) = path {
|
||||
self.data_link_manager.path = path.to_string_lossy().into_owned();
|
||||
let (sheets, named_ranges) = workbook_metadata(&path).unwrap_or_default();
|
||||
@@ -1780,7 +1828,13 @@ impl OpenCADStudio {
|
||||
// that follows this message.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
if !save {
|
||||
register_picked_source(&path.to_string_lossy(), Arc::new(handle.read().await));
|
||||
let fresh = register_picked_source(
|
||||
&path.to_string_lossy(),
|
||||
Arc::new(handle.read().await),
|
||||
);
|
||||
if !fresh {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
Some(path)
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user