diff --git a/CHANGELOG.md b/CHANGELOG.md index f58d025..655d271 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,235 @@ # Changelog +## Unreleased + +### Added + +- `H5Dataset::read_numeric_as::()` and `read_numeric_slice_as::()`: + datatype-aware numeric reads. Where `read_raw::` only checks that + `T`'s size matches the stored element size (so an `i64` read of a + `uint64` dataset reinterprets bits, and a big-endian source is misread), + these inspect the datatype message — class, signedness, byte order, + width — and convert per element. Integer→integer is checked and errors + with the element index and value instead of wrapping; `f32`→`f64` + widens exactly; `f64`→`f32`, float→integer and integer→float are + rejected. `read_raw` is unchanged. (issue #11) + +- Global-heap collections extend in place, completing the CWFS parity: + when no listed collection can take a vlen object, the writer now grows + one — the file allocation via the allocator's new `try_extend` + (end-of-file bump or consuming an adjacent released block, libhdf5's + `H5MF_try_extend`) and the collection's declared size plus free-space + marker (`H5HG_extend`) — before falling back to a fresh collection. + Growth is `max(collection_size, shortfall)` capped at the 64 KiB + `H5HG_MAXSIZE`, as upstream computes it. A small-then-big attribute + pair used to cost two collections; it now costs one. + +### Fixed + +- A v2 B-tree chunk index re-serializes at the node size its header + declares instead of this writer's compile-time 2048. Reopening a file + whose tree uses another node size (libhdf5 built with a different + `H5D_BT2_NODE_SIZE`, or any other writer) previously left the dataset + a re-link placeholder; it now reconstructs and grows, with node blocks, + the rewritten B-tree header, and the object header's layout message all + carrying the creator's node size and split/merge percentages — the + layout message used to be re-stamped with the defaults on every close. + +- Reopening a file for append now reconstructs *paged* fixed-array chunk + indexes (any fixed-shape dataset with more than 1024 chunks). The + writer has emitted the paged layout since paged-write support landed, + but `open_append` still refused it, leaving such a dataset a re-link + placeholder: writes to it failed and deleting it leaked every chunk + plus the index. Reconstruction honors the page-init bitmap — pages + libhdf5 never wrote are skipped, not decoded — so a partially written + h5py file can be completed in place. (Cross-validated both directions + against h5py, including the uninitialized-page case.) + +- `delete_dataset` and `delete_group` now follow libhdf5's `H5Ldelete` + semantics against hard links: deleting a name only unlinks it, and an + object a hard link still names survives under the link's path — + promoted to its primary name, a group bringing its whole subtree — + with storage freed only when the last name goes. Deleting a dataset + used to destroy the object and leave its hard links silently + unemitted. A path naming a hard link itself removes just that link + (previously `NotFound`, so links could never be removed). Hard links + also survive reopening the file for append — each used to come back + as an independent dataset carrying the same storage addresses, so + deleting one name freed blocks and the shared object header the other + names still referenced. And like `H5Dopen`/`H5Lcreate_hard`, an alias + path now resolves for `dataset_writer` and as a link target. + +- Paths traverse group hard links, the way HDF5 paths always resolve + through links: creating, writing, annotating, reading, and deleting + through an alias path (`root.group("inner_alias")`, + `file.dataset("inner_alias/ds")`) lands on the link's target group. + Writer-side, every path entry point canonicalizes group-link + prefixes; reader-side, the discovery walk records the alias of an + already-visited group header instead of only skipping it, and + lookups resolve through those aliases. Deletion still keeps the + *leaf* literal — `H5Ldelete` removes the named link itself, not its + target. + +- Reopening a file for appending now reads fixed-array and v2-B-tree + chunk indexes back into the writer, as it already did for extensible + arrays. Those datasets used to come back as re-link placeholders: + writes to them were refused, and deleting one freed only its + attributes and object header while every chunk block and the index + structures leaked. A paged fixed array or a v2 B-tree with a foreign + node size (neither of which this writer creates) still comes back + re-link only. + +- Vlen writes now pack their heap objects into existing global-heap + collections with free space, tracking them the way libhdf5's CWFS + list (`H5HG_insert`) does, and a batch of more than 65535 strings + spills into a further collection instead of failing with "global + heap collection is full". Every vlen write call used to allocate its + own collection, so each small string attribute or dataset paid the + 4096-byte `H5HG_MINALLOC` minimum for a block that held one value, + and one call could never exceed a single collection's 16-bit object + index space. Freed space from replaced or deleted vlen values is + re-listed for packing, so replace loops settle inside one collection. + Under SWMR active, batches keep getting fresh collections — packing + rewrites a block a streaming reader may be walking. + +- `delete_dataset` and `delete_group` now free the file space the + deleted objects owned, the way libhdf5's `H5O_delete` does: chunk + blocks and the chunk-index structures themselves (extensible-array + header/index/super/data blocks, fixed-array header and data block, + v2-B-tree header and nodes), contiguous data blocks, the global-heap + objects of variable-length data and attributes, and — on reopened + files — the on-disk object header block. They used to only unlink, so + every create/delete cycle grew the file by the object's whole + footprint, and finalize even wrote a fresh orphan header for each + deleted object. Deletes are refused while SWMR streaming is active + (a reader may hold the freed addresses; libhdf5 forbids it too), and + name resolution for `create_group` parents and dataset→group + assignment no longer matches soft-deleted groups, which could attach + a new object to a deleted parent and lose it at close. + +- SWMR attribute setters now error after `start_swmr` instead of + appearing to succeed, matching libhdf5's ban on attribute changes + during SWMR writes. Object headers are frozen once streaming starts, + so a post-start change was committed at close only when its header + happened to be rebuilt — group attributes always, dataset attributes + only if the dataset also received chunk writes — and silently dropped + otherwise; replacing a vlen attribute also stranded the superseded + value's 4096-byte heap collection forever, since a streaming reader + may still hold its references. Behavior change: calls that used to + return `Ok` now fail; set attributes before `start_swmr`. + +- `set_extent` shrinks now prune stored chunks, matching libhdf5's + `H5D__chunk_prune_by_extent`: a chunk entirely beyond the new extent is + removed from the chunk index (extensible-array, fixed-array, and v2 + B-tree) and its storage freed for reuse — it used to stay allocated and + indexed forever — and a chunk the new extent cuts through has its + out-of-extent region overwritten with the fill value. Behavior change: + growing the extent back now reads fill values where it used to resurrect + the stale pre-shrink data. Under SWMR the index entries are still + cleared but the blocks are kept, the rule libhdf5 applies in + `H5Dearray.c`. + +- Global-heap objects carry their on-disk reference count through + decode and encode. It was hardcoded to 1 on every encode — libhdf5 + writes 0 on insert (`H5HG_insert`), and rewriting a foreign + collection after an object removal reset any count its virtual-dataset + layer had raised via `H5HG_link`. New objects now encode 0, matching + libhdf5 byte-for-byte; decoded objects keep what the file declares. + +- Creating a zero-element vlen dataset (`write_vlen_strings`, + `write_vlen_bytes`, `write_vlen_strings_compressed`) or an empty + string-array attribute no longer writes a global-heap collection: an + empty collection still encodes to the 4096-byte minimum, and with no + reference pointing at it the block was orphaned. The empty dataset or + attribute itself is still created. + +- A rejected vlen write no longer orphans a global-heap collection. + `append_vlen_strings` wrote the batch's collection before checking the + dataset was chunked, and `write_vlen_strings_slice` wrote it before + the slice write could refuse a dataset with no writable storage (a + reopened fixed-array/v2-B-tree dataset, which this crate re-links but + cannot write) — every failed call grew the file by a 4096-byte block + nothing referenced. All deterministic rejections now run before the + collection is allocated. + +- Variable-length reads no longer return silent empty strings when the + global-heap collection cannot be resolved. The reader capped + collections at 64 MiB — libhdf5 has no cap, and this crate's writers + put a whole write call's strings into one collection, so reading back + a large batch blanked every string. The cap is gone, and the failures + libhdf5 treats as errors (`H5HG__cache_heap_deserialize`: bad `GCOL` + signature, declared size below 4096) are now hard errors here too, as + are a reference to an object missing from its collection and an object + index that overflows the 16-bit on-disk field. A nil reference + (address 0) still reads as the empty value. Applies to both dataset + and attribute reads, which now share one collection loader. + +- Reopening a file for writing (`open_rw` / `open_append`) now keeps + every group and its attributes. The reopen rebuilt its group registry + from dataset paths alone, so a bare `open_rw` + `close` deleted any + group with no dataset beneath it (empty, subgroup-only, or NeXus + attribute-only groups) and stripped the attributes — `NX_class` + included — off the groups that survived. Groups are now registered + from the file's link records with the attributes their headers carry, + which also lets a reopen session set attributes on a dataset-less + group. + +- A `set_extent` shrink of a variable-length dataset now releases the + global-heap objects of the elements it discards — both those in pruned + chunks and those a straddling chunk's fill refill overwrites. They used + to stay in their collections forever, so every shrink stranded at least + one 4096-byte collection; an append/shrink cycle now reuses the freed + blocks and the file size settles. This is more than libhdf5 does (its + `H5D__chunk_prune_by_extent` strands them too), matching this crate's + existing element-replace behavior. No-op under SWMR, like every other + heap release. + +- An extent change made in a reopen session with no chunk write is now + persisted at close. The finalize path inferred "modified" from the + session's chunk-write count alone, so `open_rw` + `set_extent` (or + `extend`) + `close` kept the old dataset header and silently dropped + the new shape. + +- `H5Attribute::read_numeric::` validates the stored datatype against + `T`'s before reinterpreting the value bytes. It used to transmute + unconditionally — and its length check was `<`, not `==` — so reading an + `f64` attribute as `f32` returned the low half of the double's bit image, + and big-endian, differently-classed, or even vlen-string attributes came + back as garbage values. A mismatch is now a `TypeMismatch` error; the new + `H5Attribute::read_numeric_as::()` is the converting read, with the + same checked / widening-only policy as the dataset method, and returns + every element of an array attribute. + +- `SwmrFileReader::read_dataset::` and `read_slice::` check `T` + against the dataset's stored element width. They only checked that the + byte count divided evenly, so reading an `f64` dataset as `i32` silently + returned twice as many reinterpreted values; a width mismatch is now a + `TypeMismatch` error, the same rule as `H5Dataset::read_raw`. + +- Rewriting variable-length string elements frees the superseded global-heap + objects *before* allocating the replacement collection, the order + libhdf5's `H5T__vlen_disk_write` uses, so the freed space is eligible for + immediate reuse and reopen-replace loops no longer grow the file every + session. (issue #10) + +- Replacing a variable-length string attribute releases the superseded + value's global-heap objects, before the replacement's collection is + allocated (the same free-before-alloc order as the dataset fix below), + so updating a vlen attribute — in one session or across reopen-replace + sessions — no longer strands one collection block per update. All three + attribute lists (root, group, dataset) route replacement through one + owner, which also covers a numeric value replacing a vlen one. + **Behavior change:** writing a dataset attribute whose name already + exists now replaces it, as root and group attributes (and h5py) always + did; previously the dataset header accumulated duplicate same-name + attribute messages. + +- On close, the writer frees the object-header blocks it supersedes — + root group, groups, and modified datasets are rewritten at fresh + addresses each session, and the old blocks were simply abandoned, + leaking a few dozen bytes per reopen cycle. Not done under SWMR, where + a live reader may still walk the old headers. (issue #10) + ## 0.4.2 ### Added diff --git a/src/attribute.rs b/src/attribute.rs index ff43c0b..34c1e4d 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -70,8 +70,11 @@ impl H5Attribute { let inner = borrow_inner(&self.file_inner); match &*inner { H5FileInner::Writer(writer) => { - let attr_msg = writer.vlen_string_attribute(&self.name, &value.0)?; - writer.add_dataset_attribute(self.ds_index, attr_msg)?; + writer.set_vlen_string_attribute( + crate::io::writer::AttrTarget::Dataset(self.ds_index), + &self.name, + &value.0, + )?; Ok(()) } H5FileInner::Reader(_) => Err(Hdf5Error::InvalidState( @@ -110,8 +113,12 @@ impl H5Attribute { let mut inner = borrow_inner_mut(&self.file_inner); match &mut *inner { H5FileInner::Writer(writer) => { - let attr_msg = writer.vlen_string_array_attribute(&self.name, values, &dims_u64)?; - writer.add_dataset_attribute(self.ds_index, attr_msg)?; + writer.set_vlen_string_array_attribute( + crate::io::writer::AttrTarget::Dataset(self.ds_index), + &self.name, + values, + &dims_u64, + )?; Ok(()) } H5FileInner::Reader(_) => Err(Hdf5Error::InvalidState( @@ -212,11 +219,24 @@ impl H5Attribute { /// let val: f64 = attr.read_numeric().unwrap(); /// ``` pub fn read_numeric(&self) -> Result { - let data = self + let attr = self .read_attr .as_ref() - .map(|a| &a.data) .ok_or_else(|| Hdf5Error::InvalidState("attribute has no read data".into()))?; + // The byte image is reinterpreted as `T` below, so the stored + // datatype must be exactly what `T` writes — the same message + // `write_numeric` would emit. This is what rejects reading a + // big-endian or differently-classed attribute as `T` bit-for-bit + // garbage; a converting read is `read_numeric_as`. + let expected = T::hdf5_type(); + if attr.datatype != expected { + return Err(Hdf5Error::TypeMismatch(format!( + "attribute datatype '{}' is not the requested type's datatype '{}'; \ + use read_numeric_as for a converting read or read_raw for the bytes", + attr.datatype, expected + ))); + } + let data = &attr.data; let es = T::element_size(); if data.len() < es { return Err(Hdf5Error::TypeMismatch(format!( @@ -232,6 +252,25 @@ impl H5Attribute { } } + /// Read a numeric attribute as `T`, converting each element from the + /// stored datatype — the attribute counterpart of + /// [`H5Dataset::read_numeric_as`](crate::dataset::H5Dataset::read_numeric_as). + /// + /// Returns every element (one for a scalar attribute), with the same + /// policy: integer → integer is checked and errors with the element index + /// and value instead of wrapping, `f32` → `f64` widens exactly, and + /// `f64` → `f32`, float ↔ integer, and non-numeric datatypes are + /// rejected. Big-endian sources are decoded per the stored byte order, + /// which [`read_numeric`](Self::read_numeric) refuses. + pub fn read_numeric_as(&self) -> Result> { + let attr = self + .read_attr + .as_ref() + .ok_or_else(|| Hdf5Error::InvalidState("attribute has no read data".into()))?; + let kind = crate::dataset::numeric::classify(&attr.datatype)?; + crate::dataset::numeric::convert(kind, &attr.data) + } + /// Read the attribute value as a string. /// /// Handles both fixed-length string attributes and variable-length diff --git a/src/dataset.rs b/src/dataset.rs index c3af66f..1f7cbae 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -1695,11 +1695,14 @@ impl H5Dataset { /// /// Unlike [`extend`](Self::extend), which only grows, this can reduce a /// dimension — for example to correct an over-extended frame count - /// after writing a partial multi-frame chunk. Shrinking changes the - /// logical dataspace only: data in chunks beyond the new extent stays - /// in the file but is no longer visible on read, exactly as libhdf5's - /// `H5Dset_extent` behaves. The new extent must not exceed the - /// dataset's maximum dimensions. + /// after writing a partial multi-frame chunk. Shrinking prunes the + /// stored chunks the way libhdf5's `H5Dset_extent` does: a chunk + /// entirely beyond the new extent is removed from the chunk index and + /// its storage freed for reuse, and a chunk the new extent cuts + /// through has its out-of-extent region overwritten with the fill + /// value — so growing the extent back exposes fill values, not the + /// old data. The new extent must not exceed the dataset's maximum + /// dimensions. pub fn set_extent(&self, new_dims: &[usize]) -> Result<()> { match &self.info { DatasetInfo::Writer { index, .. } => { @@ -2107,6 +2110,93 @@ impl H5Dataset { } } + /// Read a numeric dataset as `T`, converting each element from the + /// on-disk datatype. + /// + /// Unlike [`read_raw`](Self::read_raw), which requires `T`'s size to match + /// the stored element size exactly, this inspects the dataset's datatype + /// message — class, signedness, byte order, width — and converts per + /// element: + /// + /// - integer → integer: checked; a stored value that does not fit in `T` + /// is an error naming the element index and value, never a silent wrap. + /// - `f32` source → `f64`: exact widening. + /// - `f64` source → `f32`, float → integer, and integer → float are + /// rejected as [`TypeMismatch`](Hdf5Error::TypeMismatch). + /// + /// Big-endian sources are decoded according to the datatype's byte order, + /// which [`read_raw`](Self::read_raw)'s size-only check would misread. + /// + /// ```no_run + /// # use rust_hdf5::H5File; + /// let file = H5File::open("data.h5").unwrap(); + /// let ds = file.dataset("counts").unwrap(); // stored as e.g. i16 + /// let counts = ds.read_numeric_as::().unwrap(); + /// ``` + pub fn read_numeric_as(&self) -> Result> { + match &self.info { + DatasetInfo::Reader { name, .. } => { + let (kind, raw) = { + let mut inner = borrow_inner_mut(&self.file_inner); + match &mut *inner { + H5FileInner::Reader(reader) => { + let info = reader + .dataset_info(name) + .ok_or_else(|| Hdf5Error::NotFound(name.clone()))?; + let kind = numeric::classify(&info.datatype)?; + (kind, reader.read_dataset_raw(name)?) + } + _ => { + return Err(Hdf5Error::InvalidState("file is not in read mode".into())) + } + } + }; + numeric::convert(kind, &raw) + } + DatasetInfo::Writer { .. } => Err(Hdf5Error::InvalidState( + "cannot read from a dataset in write mode".into(), + )), + } + } + + /// Read a slice (hyperslab) of a numeric dataset as `T`, with the same + /// per-element datatype conversion as + /// [`read_numeric_as`](Self::read_numeric_as). + /// + /// `starts` and `counts` define the N-dimensional selection exactly as in + /// [`read_slice`](Self::read_slice). + pub fn read_numeric_slice_as( + &self, + starts: &[usize], + counts: &[usize], + ) -> Result> { + match &self.info { + DatasetInfo::Reader { name, .. } => { + let starts_u64: Vec = starts.iter().map(|&s| s as u64).collect(); + let counts_u64: Vec = counts.iter().map(|&c| c as u64).collect(); + let (kind, raw) = { + let mut inner = borrow_inner_mut(&self.file_inner); + match &mut *inner { + H5FileInner::Reader(reader) => { + let info = reader + .dataset_info(name) + .ok_or_else(|| Hdf5Error::NotFound(name.clone()))?; + let kind = numeric::classify(&info.datatype)?; + (kind, reader.read_slice(name, &starts_u64, &counts_u64)?) + } + _ => { + return Err(Hdf5Error::InvalidState("file is not in read mode".into())) + } + } + }; + numeric::convert(kind, &raw) + } + DatasetInfo::Writer { .. } => Err(Hdf5Error::InvalidState( + "cannot read_slice from a dataset in write mode".into(), + )), + } + } + /// Read the whole dataset into a caller-provided buffer, with no allocation. /// /// `out` must have exactly `product(dims)` elements (the dataset's element @@ -2219,6 +2309,247 @@ impl H5Dataset { } } +// --------------------------------------------------------------------------- +// Datatype-aware numeric conversion (read_numeric_as) +// --------------------------------------------------------------------------- + +/// Marker trait for the Rust types [`H5Dataset::read_numeric_as`] can convert +/// into: the integer primitives (checked, never wrapping) plus `f32`/`f64` +/// (widening only). +/// +/// Sealed — the conversion policy is part of the library contract, so the +/// trait cannot be implemented outside this crate. +pub trait ReadNumeric: numeric::Sealed {} +impl ReadNumeric for T {} + +pub(crate) mod numeric { + //! Per-element decode + checked conversion for `read_numeric_as` (and the + //! attribute counterpart `H5Attribute::read_numeric_as`). + + use crate::error::{Hdf5Error, Result}; + use crate::format::messages::datatype::{ByteOrder, DatatypeMessage}; + + /// A source element, normalized: every standard integer width — u64::MAX + /// included — fits in `i128` without loss. + pub enum NumericSource { + Int(i128), + F32(f32), + F64(f64), + } + + /// The on-disk element shape `classify` accepted. + #[derive(Clone, Copy)] + pub enum SourceKind { + Int { + size: usize, + signed: bool, + byte_order: ByteOrder, + }, + F32(ByteOrder), + F64(ByteOrder), + } + + impl SourceKind { + fn element_size(self) -> usize { + match self { + SourceKind::Int { size, .. } => size, + SourceKind::F32(_) => 4, + SourceKind::F64(_) => 8, + } + } + } + + /// Map a datatype message to a supported numeric source shape. + /// + /// Accepts standard-width integers (1/2/4/8 bytes, full precision, zero + /// bit offset) and IEEE binary32/binary64 floats; everything else is a + /// `TypeMismatch` naming what was found. + pub fn classify(dt: &DatatypeMessage) -> Result { + match *dt { + DatatypeMessage::FixedPoint { + size, + byte_order, + signed, + bit_offset, + bit_precision, + } => { + if !matches!(size, 1 | 2 | 4 | 8) + || bit_offset != 0 + || u32::from(bit_precision) != size * 8 + { + return Err(Hdf5Error::TypeMismatch(format!( + "fixed-point datatype (size {size}, bit offset {bit_offset}, \ + precision {bit_precision}) is not a standard-width integer", + ))); + } + Ok(SourceKind::Int { + size: size as usize, + signed, + byte_order, + }) + } + DatatypeMessage::FloatingPoint { + size, + byte_order, + exponent_size, + mantissa_size, + .. + } => match (size, exponent_size, mantissa_size) { + (4, 8, 23) => Ok(SourceKind::F32(byte_order)), + (8, 11, 52) => Ok(SourceKind::F64(byte_order)), + _ => Err(Hdf5Error::TypeMismatch(format!( + "floating-point datatype (size {size}, exponent {exponent_size} bits, \ + mantissa {mantissa_size} bits) is not IEEE binary32 or binary64", + ))), + }, + ref other => Err(Hdf5Error::TypeMismatch(format!( + "dataset datatype '{other}' is not numeric", + ))), + } + } + + fn decode_element(kind: SourceKind, bytes: &[u8]) -> NumericSource { + match kind { + SourceKind::Int { + size, + signed, + byte_order, + } => { + let mut le = [0u8; 8]; + match byte_order { + ByteOrder::LittleEndian => le[..size].copy_from_slice(bytes), + ByteOrder::BigEndian => { + for (dst, src) in le[..size].iter_mut().zip(bytes.iter().rev()) { + *dst = *src; + } + } + } + let zero_extended = u64::from_le_bytes(le); + let value = if signed { + // Arithmetic right shift sign-extends the low `size` bytes. + let shift = 64 - 8 * size as u32; + i128::from(((zero_extended as i64) << shift) >> shift) + } else { + i128::from(zero_extended) + }; + NumericSource::Int(value) + } + SourceKind::F32(byte_order) => { + let arr: [u8; 4] = bytes.try_into().unwrap(); + NumericSource::F32(match byte_order { + ByteOrder::LittleEndian => f32::from_le_bytes(arr), + ByteOrder::BigEndian => f32::from_be_bytes(arr), + }) + } + SourceKind::F64(byte_order) => { + let arr: [u8; 8] = bytes.try_into().unwrap(); + NumericSource::F64(match byte_order { + ByteOrder::LittleEndian => f64::from_le_bytes(arr), + ByteOrder::BigEndian => f64::from_be_bytes(arr), + }) + } + } + } + + /// Decode and convert every element of `raw` into `T`. + pub fn convert(kind: SourceKind, raw: &[u8]) -> Result> { + let size = kind.element_size(); + if !raw.len().is_multiple_of(size) { + return Err(Hdf5Error::TypeMismatch(format!( + "raw data size {} is not a multiple of element size {size}", + raw.len(), + ))); + } + raw.chunks_exact(size) + .enumerate() + .map(|(index, bytes)| T::from_source(decode_element(kind, bytes), index)) + .collect() + } + + /// The sealed half of `ReadNumeric`: how one normalized source element + /// becomes a `Self`, or a `TypeMismatch` explaining why it cannot. + pub trait Sealed: Sized { + fn from_source(src: NumericSource, index: usize) -> Result; + } + + macro_rules! int_targets { + ($($t:ty),* $(,)?) => {$( + impl Sealed for $t { + fn from_source(src: NumericSource, index: usize) -> Result { + match src { + NumericSource::Int(v) => <$t>::try_from(v).map_err(|_| { + Hdf5Error::TypeMismatch(format!( + concat!( + "value {} at element {} does not fit in ", + stringify!($t), + ), + v, index, + )) + }), + NumericSource::F32(_) | NumericSource::F64(_) => { + Err(Hdf5Error::TypeMismatch( + concat!( + "cannot read a floating-point dataset as ", + stringify!($t), + "; read as f64 and convert explicitly", + ) + .into(), + )) + } + } + } + } + )*}; + } + int_targets!(i8, i16, i32, i64, u8, u16, u32, u64, u128); + + // Not in the macro: `i128::try_from(i128)` is infallible, which trips + // clippy::unnecessary_fallible_conversions. + impl Sealed for i128 { + fn from_source(src: NumericSource, _index: usize) -> Result { + match src { + NumericSource::Int(v) => Ok(v), + NumericSource::F32(_) | NumericSource::F64(_) => Err(Hdf5Error::TypeMismatch( + "cannot read a floating-point dataset as i128; read as f64 and \ + convert explicitly" + .into(), + )), + } + } + } + + impl Sealed for f32 { + fn from_source(src: NumericSource, _index: usize) -> Result { + match src { + NumericSource::F32(v) => Ok(v), + NumericSource::F64(_) => Err(Hdf5Error::TypeMismatch( + "narrowing an f64 dataset to f32 loses precision; read as f64".into(), + )), + NumericSource::Int(_) => Err(Hdf5Error::TypeMismatch( + "cannot read an integer dataset as f32; read as an integer type and \ + convert explicitly" + .into(), + )), + } + } + } + + impl Sealed for f64 { + fn from_source(src: NumericSource, _index: usize) -> Result { + match src { + NumericSource::F64(v) => Ok(v), + // Every f32 is exactly representable as f64. + NumericSource::F32(v) => Ok(f64::from(v)), + NumericSource::Int(_) => Err(Hdf5Error::TypeMismatch( + "cannot read an integer dataset as f64; integers above 2^53 lose \ + precision — read as an integer type and convert explicitly" + .into(), + )), + } + } + } +} + #[cfg(test)] mod tests { use crate::H5File; @@ -5563,4 +5894,277 @@ mod tests { file.close().unwrap(); std::fs::remove_file(&path).ok(); } + + /// Boundary: `fits in T` vs `does not fit in T`, for both the too-large + /// (u64::MAX → i64) and the negative-to-unsigned (−1 → u32) directions. + #[test] + fn numeric_int_checked_conversion_boundaries() { + let path = temp_path("numeric_int_bounds"); + { + let file = H5File::create(&path).unwrap(); + let ds = file.new_dataset::().shape([2]).create("u").unwrap(); + ds.write_raw(&[1u64, u64::MAX]).unwrap(); + let ds = file.new_dataset::().shape([2]).create("i").unwrap(); + ds.write_raw(&[-1i32, 5]).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + + let u = file.dataset("u").unwrap(); + assert_eq!(u.read_numeric_as::().unwrap(), vec![1, u64::MAX]); + assert_eq!( + u.read_numeric_as::().unwrap(), + vec![1, i128::from(u64::MAX)] + ); + let err = u.read_numeric_as::().unwrap_err(); + assert!( + err.to_string() + .contains("value 18446744073709551615 at element 1 does not fit in i64"), + "unexpected error: {err}" + ); + + let i = file.dataset("i").unwrap(); + assert_eq!(i.read_numeric_as::().unwrap(), vec![-1, 5]); + let err = i.read_numeric_as::().unwrap_err(); + assert!( + err.to_string() + .contains("value -1 at element 0 does not fit in u32"), + "unexpected error: {err}" + ); + std::fs::remove_file(&path).ok(); + } + + /// Boundary: f32 → f64 is exact widening; f64 → f32 is rejected. + #[test] + fn numeric_float_widening_only() { + let path = temp_path("numeric_float"); + { + let file = H5File::create(&path).unwrap(); + let ds = file.new_dataset::().shape([2]).create("f4").unwrap(); + ds.write_raw(&[1.5f32, -2.25]).unwrap(); + let ds = file.new_dataset::().shape([1]).create("f8").unwrap(); + ds.write_raw(&[3.75f64]).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + + let f4 = file.dataset("f4").unwrap(); + assert_eq!(f4.read_numeric_as::().unwrap(), vec![1.5, -2.25]); + assert_eq!(f4.read_numeric_as::().unwrap(), vec![1.5, -2.25]); + + let f8 = file.dataset("f8").unwrap(); + assert_eq!(f8.read_numeric_as::().unwrap(), vec![3.75]); + let err = f8.read_numeric_as::().unwrap_err(); + assert!( + err.to_string().contains("narrowing"), + "unexpected error: {err}" + ); + std::fs::remove_file(&path).ok(); + } + + /// Boundary: cross-class conversions (float ↔ integer) are rejected in + /// both directions, and a non-numeric datatype is rejected at classify. + #[test] + fn numeric_cross_class_and_non_numeric_rejected() { + let path = temp_path("numeric_cross_class"); + { + let file = H5File::create(&path).unwrap(); + let ds = file.new_dataset::().shape([1]).create("f8").unwrap(); + ds.write_raw(&[1.0f64]).unwrap(); + let ds = file.new_dataset::().shape([1]).create("i4").unwrap(); + ds.write_raw(&[7i32]).unwrap(); + file.write_vlen_strings("s", &["a", "b"]).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + + let err = file + .dataset("f8") + .unwrap() + .read_numeric_as::() + .unwrap_err(); + assert!( + err.to_string().contains("floating-point dataset as i64"), + "unexpected error: {err}" + ); + let err = file + .dataset("i4") + .unwrap() + .read_numeric_as::() + .unwrap_err(); + assert!( + err.to_string().contains("integer dataset as f64"), + "unexpected error: {err}" + ); + let err = file + .dataset("s") + .unwrap() + .read_numeric_as::() + .unwrap_err(); + assert!( + err.to_string().contains("is not numeric"), + "unexpected error: {err}" + ); + std::fs::remove_file(&path).ok(); + } + + /// Boundary: big-endian sources decode per the datatype's byte order. + /// Unit-level (the writer only emits little-endian): feed `convert` a + /// big-endian datatype plus big-endian bytes directly. + #[test] + fn numeric_big_endian_decode() { + use super::numeric; + use crate::format::messages::datatype::{ByteOrder, DatatypeMessage}; + + let dt = DatatypeMessage::FixedPoint { + size: 4, + byte_order: ByteOrder::BigEndian, + signed: true, + bit_offset: 0, + bit_precision: 32, + }; + let mut raw = Vec::new(); + raw.extend_from_slice(&(-2i32).to_be_bytes()); + raw.extend_from_slice(&(100_000i32).to_be_bytes()); + let kind = numeric::classify(&dt).unwrap(); + assert_eq!( + numeric::convert::(kind, &raw).unwrap(), + vec![-2, 100_000] + ); + + let dt = DatatypeMessage::FloatingPoint { + size: 8, + byte_order: ByteOrder::BigEndian, + sign_location: 63, + bit_offset: 0, + bit_precision: 64, + exponent_location: 52, + exponent_size: 11, + mantissa_location: 0, + mantissa_size: 52, + exponent_bias: 1023, + }; + let raw = (-2.25f64).to_be_bytes(); + let kind = numeric::classify(&dt).unwrap(); + assert_eq!(numeric::convert::(kind, &raw).unwrap(), vec![-2.25]); + } + + /// `H5Attribute::read_numeric` validates the stored datatype before + /// reinterpreting bytes: cross-width, cross-class, and non-numeric + /// attributes error instead of returning bit-garbage, while the exact + /// type and the HBool / complex-compound paths keep working. + #[test] + fn attr_read_numeric_validates_datatype() { + use crate::types::{Complex64, HBool, VarLenUnicode}; + let path = temp_path("attr_read_numeric_validate"); + { + let file = H5File::create(&path).unwrap(); + let ds = file.new_dataset::().shape([2]).create("d").unwrap(); + ds.write_raw(&[1.0f32; 2]).unwrap(); + let a = ds.new_attr::().shape(()).create("f8").unwrap(); + a.write_numeric(&1.5f64).unwrap(); + let a = ds.new_attr::().shape(()).create("i4").unwrap(); + a.write_numeric(&-7i32).unwrap(); + let a = ds.new_attr::().shape(()).create("b").unwrap(); + a.write_numeric(&HBool::from(true)).unwrap(); + let a = ds.new_attr::().shape(()).create("z").unwrap(); + a.write_numeric(&Complex64 { re: 1.0, im: -2.0 }).unwrap(); + let a = ds + .new_attr::() + .shape(()) + .create("s") + .unwrap(); + a.write_scalar(&VarLenUnicode("text".into())).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + + let f8 = ds.attr("f8").unwrap(); + assert_eq!(f8.read_numeric::().unwrap(), 1.5); + // Previously returned the low half of the f64 image as an f32. + let err = f8.read_numeric::().unwrap_err(); + assert!( + err.to_string().contains("read_numeric_as"), + "unexpected error: {err}" + ); + assert!(f8.read_numeric::().is_err()); + + let i4 = ds.attr("i4").unwrap(); + assert_eq!(i4.read_numeric::().unwrap(), -7); + assert!(i4.read_numeric::().is_err()); + + assert!(bool::from( + ds.attr("b").unwrap().read_numeric::().unwrap() + )); + let z = ds.attr("z").unwrap().read_numeric::().unwrap(); + assert_eq!((z.re, z.im), (1.0, -2.0)); + + // A vlen string attribute: read_numeric used to transmute the heap + // reference bytes into the requested type. + let s = ds.attr("s").unwrap(); + assert!(s.read_numeric::().is_err()); + assert!(s.read_numeric_as::().is_err()); + std::fs::remove_file(&path).ok(); + } + + /// The attribute conversion read applies the dataset rules: checked + /// int → int naming index and value on overflow, widening-only floats, + /// cross-class rejected; an array attribute converts every element. + #[test] + fn attr_read_numeric_as_converts() { + let path = temp_path("attr_read_numeric_as"); + { + let file = H5File::create(&path).unwrap(); + let ds = file.new_dataset::().shape([2]).create("d").unwrap(); + ds.write_raw(&[1.0f32; 2]).unwrap(); + let a = ds.new_attr::().shape(()).create("i4").unwrap(); + a.write_numeric(&-7i32).unwrap(); + let a = ds.new_attr::().shape(()).create("u8max").unwrap(); + a.write_numeric(&u64::MAX).unwrap(); + let a = ds.new_attr::().shape([3]).create("arr").unwrap(); + a.write_array(&[1i16, -2, 3]).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!( + ds.attr("i4").unwrap().read_numeric_as::().unwrap(), + vec![-7] + ); + let err = ds + .attr("u8max") + .unwrap() + .read_numeric_as::() + .unwrap_err(); + assert!( + err.to_string().contains("does not fit in i64"), + "unexpected error: {err}" + ); + assert_eq!( + ds.attr("arr").unwrap().read_numeric_as::().unwrap(), + vec![1, -2, 3] + ); + assert!(ds.attr("i4").unwrap().read_numeric_as::().is_err()); + std::fs::remove_file(&path).ok(); + } + + /// The hyperslab variant applies the same conversion to a sub-selection. + #[test] + fn numeric_slice_conversion() { + let path = temp_path("numeric_slice"); + { + let file = H5File::create(&path).unwrap(); + let ds = file.new_dataset::().shape([2, 3]).create("m").unwrap(); + ds.write_raw(&[1i16, 2, 3, 4, 5, 6]).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + let m = file.dataset("m").unwrap(); + assert_eq!( + m.read_numeric_slice_as::(&[0, 1], &[2, 2]).unwrap(), + vec![2, 3, 5, 6] + ); + std::fs::remove_file(&path).ok(); + } } diff --git a/src/file.rs b/src/file.rs index cf175df..8394ca8 100644 --- a/src/file.rs +++ b/src/file.rs @@ -232,8 +232,11 @@ impl H5File { let inner = borrow_inner(&self.inner); match &*inner { H5FileInner::Writer(writer) => { - let attr = writer.vlen_string_attribute(name, value)?; - writer.add_root_attribute(attr); + writer.set_vlen_string_attribute( + crate::io::writer::AttrTarget::Root, + name, + value, + )?; Ok(()) } _ => Err(Hdf5Error::InvalidState("cannot write in read mode".into())), @@ -249,7 +252,7 @@ impl H5File { let inner = borrow_inner(&self.inner); match &*inner { H5FileInner::Writer(writer) => { - writer.add_root_attribute(attr); + writer.add_root_attribute(attr)?; Ok(()) } _ => Err(Hdf5Error::InvalidState("cannot write in read mode".into())), @@ -302,7 +305,7 @@ impl H5File { let mut inner = borrow_inner_mut(&self.inner); match &mut *inner { H5FileInner::Writer(writer) => { - writer.add_root_attribute(attr); + writer.add_root_attribute(attr)?; Ok(()) } _ => Err(Hdf5Error::InvalidState("cannot write in read mode".into())), @@ -343,8 +346,12 @@ impl H5File { let mut inner = borrow_inner_mut(&self.inner); match &mut *inner { H5FileInner::Writer(writer) => { - let attr = writer.vlen_string_array_attribute(name, values, &dims)?; - writer.add_root_attribute(attr); + writer.set_vlen_string_array_attribute( + crate::io::writer::AttrTarget::Root, + name, + values, + &dims, + )?; Ok(()) } _ => Err(Hdf5Error::InvalidState("cannot write in read mode".into())), @@ -566,8 +573,14 @@ impl H5File { } } - /// Delete a dataset by name. The dataset is unlinked on close; - /// file space is not reclaimed. + /// Delete a dataset name, with libhdf5's `H5Ldelete` semantics: a + /// path naming a hard link removes just that link, and if a hard link + /// still names the object whose tree name is deleted, the dataset + /// lives on under the link. Deleting the last name unlinks the + /// dataset on close and the file space it owned — data blocks, + /// chunk-index structures, and the global-heap objects of + /// variable-length values — is freed for reuse by later writes in this + /// session (the file itself does not shrink). pub fn delete_dataset(&self, name: &str) -> Result<()> { let inner = borrow_inner(&self.inner); match &*inner { @@ -579,8 +592,14 @@ impl H5File { } } - /// Delete a group and all its child datasets/sub-groups. - /// File space is not reclaimed. + /// Delete a group and all its child datasets/sub-groups, freeing their + /// file space the way [`delete_dataset`](Self::delete_dataset) does. + /// + /// Hard links reaching in from outside the deleted subtree keep their + /// targets alive: a dataset or group named by such a link survives + /// under the link's path (a group brings its whole subtree with it), + /// and a `name` that is itself a hard link's path removes just that + /// link. pub fn delete_group(&self, name: &str) -> Result<()> { let inner = borrow_inner(&self.inner); match &*inner { diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index 1134c97..a9d45ca 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -710,6 +710,11 @@ pub struct Bt2Tree { pub record_size: u16, /// Size of every node in bytes. pub node_size: u32, + /// Split percentage the header declares (pass-through; see + /// [`Bt2ChunkIndex::split_percent`]). + pub split_percent: u8, + /// Merge percentage the header declares (pass-through). + pub merge_percent: u8, /// Node geometry for depths `0..=depth()`. pub geometry: Bt2Geometry, } @@ -803,6 +808,8 @@ impl Bt2Tree { record_type, record_size, node_size, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, } } @@ -889,8 +896,8 @@ impl Bt2Tree { node_size: self.node_size, record_size: self.record_size, depth: self.depth(), - split_percent: BT2_SPLIT_PERCENT, - merge_percent: BT2_MERGE_PERCENT, + split_percent: self.split_percent, + merge_percent: self.merge_percent, root_node_addr: if self.nodes.is_empty() { UNDEF_ADDR } else { @@ -934,6 +941,17 @@ pub struct Bt2ChunkIndex { /// Width in bytes of a filtered record's compressed-size field. Meaningful /// only when `filtered`; see [`compute_chunk_size_len`]. pub chunk_size_len: u8, + /// Node size every (re-)serialization of this index uses: [`BT2_NODE_SIZE`] + /// for a tree this writer creates, the on-disk header's value for a + /// reopened tree. libhdf5 sizes every node from `hdr->node_size` + /// (`H5B2leaf.c`, `H5B2internal.c`), never from a compile-time constant. + pub node_size: u32, + /// Split percentage carried into the header. Advisory for this index — + /// the bulk loader rebuilds whole trees instead of splitting nodes — but + /// a reopened tree must hand back the value its creator declared. + pub split_percent: u8, + /// Merge percentage carried into the header (advisory, as above). + pub merge_percent: u8, } impl Bt2ChunkIndex { @@ -945,6 +963,9 @@ impl Bt2ChunkIndex { records: Vec::new(), filtered_records: Vec::new(), chunk_size_len: 0, + node_size: BT2_NODE_SIZE, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, } } @@ -959,6 +980,9 @@ impl Bt2ChunkIndex { records: Vec::new(), filtered_records: Vec::new(), chunk_size_len, + node_size: BT2_NODE_SIZE, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, } } @@ -1100,19 +1124,22 @@ impl Bt2ChunkIndex { } } - /// Bulk-load these records into a v2 B-tree of [`BT2_NODE_SIZE`]-byte - /// nodes. + /// Bulk-load these records into a v2 B-tree of + /// [`node_size`](Self::node_size)-byte nodes. /// /// The records are already in key order (see the type docs), which is /// exactly what [`Bt2Tree::build`] needs. pub fn build_tree(&self, ctx: &FormatContext) -> Bt2Tree { - Bt2Tree::build( + let mut tree = Bt2Tree::build( self.record_type(), self.record_size(ctx), - BT2_NODE_SIZE, + self.node_size, ctx.sizeof_addr, &self.encode_records(ctx), - ) + ); + tree.split_percent = self.split_percent; + tree.merge_percent = self.merge_percent; + tree } /// Decode unfiltered records from a leaf node's raw record data. @@ -1779,6 +1806,40 @@ mod tests { assert_eq!(walked, idx.encode_records(&ctx)); } + /// The bulk load sizes nodes from the index's `node_size`, not the + /// compile-time default — a reopened foreign tree re-serializes at the + /// size its header declares, the way libhdf5 allocates every node at + /// `hdr->node_size`. Split/merge pass through to the header the same way. + #[test] + fn bulk_load_honors_a_foreign_node_size() { + let ctx = ctx8(); + // record_size 24, node 512: a leaf holds (512 - 10) / 24 = 20, so + // 200 records force at least one internal level. + let mut idx = index_with(200); + idx.node_size = 512; + idx.split_percent = 90; + idx.merge_percent = 30; + + let tree = idx.build_tree(&ctx); + assert_eq!(tree.node_size, 512); + assert!( + tree.depth() >= 1, + "200 records must not fit one 512-byte leaf" + ); + let addrs: Vec = (0..tree.nodes.len() as u64) + .map(|i| 0x1000 + i * 512) + .collect(); + for image in tree.encode(&ctx, &addrs) { + assert_eq!(image.len(), 512, "every node image fills its block"); + } + + let (hdr, walked) = serialize_and_walk(&idx, &ctx); + assert_eq!(hdr.node_size, 512); + assert_eq!(hdr.split_percent, 90); + assert_eq!(hdr.merge_percent, 30); + assert_eq!(walked, idx.encode_records(&ctx)); + } + /// The record count a node reports is a u16; a tree that kept every record /// in one node would silently truncate past 65535. Splitting keeps every /// node small, so the only count that has to be wide is the header's diff --git a/src/format/global_heap.rs b/src/format/global_heap.rs index 5da23df..54f744d 100644 --- a/src/format/global_heap.rs +++ b/src/format/global_heap.rs @@ -36,11 +36,24 @@ const GCOL_VERSION: u8 = 1; /// Minimum collection size required by the HDF5 C library (H5HG_MINALLOC). const GCOL_MIN_SIZE: usize = 4096; +/// Ceiling to which the CWFS second pass grows a collection in place +/// (`H5HG_MAXSIZE`): `H5F_cwfs_find_free_heap` extends a listed collection +/// only while `size + new_need <= H5HG_MAXSIZE`. Collections *created* +/// larger than this (one oversized object) are legal — they are just never +/// extended. +pub const GCOL_MAX_SIZE: usize = 65536; + /// A single object within a global heap collection. #[derive(Debug, Clone, PartialEq, Eq)] pub struct GlobalHeapObject { /// Object index (1-based). Index 0 is reserved for the free-space marker. pub index: u16, + /// On-disk reference count. libhdf5 writes 0 on insert (`H5HG_insert`) + /// and only its virtual-dataset layer ever raises it via `H5HG_link`, + /// so new objects carry 0 — but a decoded object keeps what the file + /// says, or rewriting a foreign collection after a removal would reset + /// a VDS-linked object's count. + pub ref_count: u16, /// Raw data stored in this object. pub data: Vec, } @@ -71,7 +84,11 @@ impl GlobalHeapCollection { )); } let index = max_index + 1; - self.objects.push(GlobalHeapObject { index, data }); + self.objects.push(GlobalHeapObject { + index, + ref_count: 0, + data, + }); Ok(index) } @@ -110,6 +127,32 @@ impl GlobalHeapCollection { self.objects.is_empty() } + /// The bytes the free-space marker owns (its own 16-byte header + /// included) when this collection is encoded into a `collection_size` + /// block, or `None` when the objects (plus that marker header) do not + /// fit — the fit test [`encode_at_size`](Self::encode_at_size) applies. + pub fn free_space_at(&self, ctx: &FormatContext, collection_size: usize) -> Option { + let (header_size, objhdr_size, objects_size) = self.layout(ctx); + let content_size = header_size + objects_size + objhdr_size; + if collection_size < content_size { + return None; + } + Some(collection_size - header_size - objects_size) + } + + /// The bytes an object of `data_len` occupies on disk: its aligned + /// header plus the 8-byte-aligned data. What one insert takes from a + /// collection's free space. + pub fn object_disk_size(ctx: &FormatContext, data_len: usize) -> usize { + let ss = ctx.sizeof_size as usize; + pad_to_8(2 + 2 + 4 + ss) + pad_to_8(data_len) + } + + /// The highest object index in use (0 when the collection is empty). + pub fn max_index(&self) -> u16 { + self.objects.iter().map(|o| o.index).max().unwrap_or(0) + } + /// Encode the collection into a byte vector. /// /// The encoded blob includes the GCOL header and all heap objects, @@ -169,7 +212,7 @@ impl GlobalHeapCollection { for obj in &self.objects { let obj_start = buf.len(); buf.extend_from_slice(&obj.index.to_le_bytes()); - buf.extend_from_slice(&1u16.to_le_bytes()); // ref_count = 1 + buf.extend_from_slice(&obj.ref_count.to_le_bytes()); buf.extend_from_slice(&0u32.to_le_bytes()); // reserved buf.extend_from_slice(&(obj.data.len() as u64).to_le_bytes()[..ss]); buf.resize(obj_start + objhdr_size, 0); // pad object header @@ -278,7 +321,7 @@ impl GlobalHeapCollection { let obj_start = pos; let index = u16::from_le_bytes([buf[pos], buf[pos + 1]]); pos += 2; - let _ref_count = u16::from_le_bytes([buf[pos], buf[pos + 1]]); + let ref_count = u16::from_le_bytes([buf[pos], buf[pos + 1]]); pos += 2; let _reserved = u32::from_le_bytes([buf[pos], buf[pos + 1], buf[pos + 2], buf[pos + 3]]); @@ -310,7 +353,11 @@ impl GlobalHeapCollection { let padded = pad_to_8(size); pos += padded; - objects.push(GlobalHeapObject { index, data }); + objects.push(GlobalHeapObject { + index, + ref_count, + data, + }); } Ok((Self { objects }, collection_size)) @@ -624,4 +671,38 @@ mod tests { // Asking for less than the objects need is refused, not truncated. assert!(coll.encode_at_size(&ctx(), 64).is_err()); } + + /// New objects encode a zero reference count, matching `H5HG_insert` + /// (`UINT16ENCODE(p, 0)` — only the virtual-dataset layer's `H5HG_link` + /// ever raises it). It used to be hardcoded to 1. + #[test] + fn new_objects_encode_a_zero_ref_count() { + let mut coll = GlobalHeapCollection::new(); + coll.add_object(b"x".to_vec()).unwrap(); + let img = coll.encode(&ctx()); + let header_size = pad_to_8(4 + 1 + 3 + ctx().sizeof_size as usize); + // Object header: index (2) then ref_count (2). + assert_eq!(&img[header_size + 2..header_size + 4], &[0, 0]); + } + + /// A decoded object keeps the reference count the file declares, and a + /// removal's rewrite re-encodes it unchanged — resetting it would strip + /// a foreign file's VDS link count from the surviving objects. + #[test] + fn rewrite_preserves_a_foreign_objects_ref_count() { + let mut coll = GlobalHeapCollection::new(); + coll.add_object(b"doomed".to_vec()).unwrap(); + coll.add_object(b"vds linked".to_vec()).unwrap(); + coll.objects[1].ref_count = 3; + let img = coll.encode(&ctx()); + + let (mut back, _) = GlobalHeapCollection::decode(&img, &ctx()).unwrap(); + assert_eq!(back.objects[1].ref_count, 3, "decode must keep the count"); + + assert!(back.remove_object(1)); + let rewritten = back.encode_at_size(&ctx(), img.len()).unwrap(); + let (again, _) = GlobalHeapCollection::decode(&rewritten, &ctx()).unwrap(); + assert_eq!(again.get_object(2), Some(b"vds linked".as_slice())); + assert_eq!(again.objects[0].ref_count, 3, "rewrite must keep the count"); + } } diff --git a/src/format/messages/data_layout.rs b/src/format/messages/data_layout.rs index 144d82a..ca74865 100644 --- a/src/format/messages/data_layout.rs +++ b/src/format/messages/data_layout.rs @@ -109,6 +109,34 @@ impl FixedArrayParams { } } +/// Parameters for the v2 B-tree chunk index (node size, split/merge +/// percentages — libhdf5's creation `cparam`). The v2 B-tree header carries +/// authoritative copies; libhdf5 reads these only at creation, but a +/// rewritten object header must not contradict the header of the tree it +/// points at. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Bt2Params { + pub node_size: u32, + pub split_percent: u8, + pub merge_percent: u8, +} + +impl Bt2Params { + /// This writer's creation defaults, matching libhdf5's + /// `H5D_BT2_NODE_SIZE` / `H5D_BT2_SPLIT_PERC` / `H5D_BT2_MERGE_PERC` + /// (`H5Dpkg.h`). + pub fn default_params() -> Self { + use crate::format::chunk_index::btree_v2::{ + BT2_MERGE_PERCENT, BT2_NODE_SIZE, BT2_SPLIT_PERCENT, + }; + Self { + node_size: BT2_NODE_SIZE, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, + } + } +} + /// Filtered single-chunk index parameters. /// /// When a version-4 chunked layout uses the Single Chunk index AND the @@ -171,6 +199,8 @@ pub enum DataLayoutMessage { earray_params: Option, /// Fixed array parameters (present when index_type == FixedArray). farray_params: Option, + /// v2 B-tree parameters (present when index_type == BTreeV2). + bt2_params: Option, /// Filtered single-chunk parameters (present when index_type == /// SingleChunk and the layout's filtered flag `0x02` is set). single_chunk_filter: Option, @@ -226,6 +256,7 @@ impl DataLayoutMessage { index_type: ChunkIndexType::ExtensibleArray, earray_params: Some(earray_params), farray_params: None, + bt2_params: None, single_chunk_filter: None, index_address, } @@ -247,6 +278,7 @@ impl DataLayoutMessage { index_type: ChunkIndexType::FixedArray, earray_params: None, farray_params: Some(farray_params), + bt2_params: None, single_chunk_filter: None, index_address, } @@ -255,7 +287,12 @@ impl DataLayoutMessage { /// Version 4 chunked layout with B-tree v2 index. /// /// `chunk_dims` should include the trailing element-size dimension. - pub fn chunked_v4_btree_v2(version: u8, chunk_dims: Vec, index_address: u64) -> Self { + pub fn chunked_v4_btree_v2( + version: u8, + chunk_dims: Vec, + bt2_params: Bt2Params, + index_address: u64, + ) -> Self { Self::ChunkedV4 { version, flags: 0, @@ -263,6 +300,7 @@ impl DataLayoutMessage { index_type: ChunkIndexType::BTreeV2, earray_params: None, farray_params: None, + bt2_params: Some(bt2_params), single_chunk_filter: None, index_address, } @@ -279,6 +317,7 @@ impl DataLayoutMessage { index_type: ChunkIndexType::SingleChunk, earray_params: None, farray_params: None, + bt2_params: None, single_chunk_filter: None, index_address, } @@ -330,6 +369,7 @@ impl DataLayoutMessage { index_type, earray_params, farray_params, + bt2_params, single_chunk_filter, index_address, } => { @@ -375,13 +415,15 @@ impl DataLayoutMessage { } ChunkIndexType::BTreeV2 => { // node_size(4) + split_percent(1) + merge_percent(1), - // the same geometry the B-tree header carries. - use crate::format::chunk_index::btree_v2::{ - BT2_MERGE_PERCENT, BT2_NODE_SIZE, BT2_SPLIT_PERCENT, - }; - buf.extend_from_slice(&BT2_NODE_SIZE.to_le_bytes()); - buf.push(BT2_SPLIT_PERCENT); - buf.push(BT2_MERGE_PERCENT); + // the same geometry the B-tree header carries — the + // message must agree with the BTHD it points at, so + // a reopened foreign node size is preserved, not + // stamped over with this writer's default. + if let Some(ref params) = bt2_params { + buf.extend_from_slice(¶ms.node_size.to_le_bytes()); + buf.push(params.split_percent); + buf.push(params.merge_percent); + } } // A filtered single chunk carries its on-disk size // (sizeof_size bytes) and 4-byte filter mask inline, before @@ -587,6 +629,7 @@ impl DataLayoutMessage { // Index-type-specific parameters let mut earray_params = None; let mut farray_params = None; + let mut bt2_params = None; let mut single_chunk_filter = None; match index_type { @@ -637,14 +680,25 @@ impl DataLayoutMessage { } ChunkIndexType::BTreeV2 => { // node_size(4) + split_percent(1) + merge_percent(1). - // The v2 B-tree header carries authoritative copies, - // so the reader only needs to skip these. + // The v2 B-tree header carries authoritative copies; + // retained so a rewritten object header re-emits the + // creator's values, not this writer's defaults. if buf.len() < pos + 6 { return Err(FormatError::BufferTooShort { needed: pos + 6, available: buf.len(), }); } + bt2_params = Some(Bt2Params { + node_size: u32::from_le_bytes([ + buf[pos], + buf[pos + 1], + buf[pos + 2], + buf[pos + 3], + ]), + split_percent: buf[pos + 4], + merge_percent: buf[pos + 5], + }); pos += 6; } // A single-chunk index whose "single index with @@ -699,6 +753,7 @@ impl DataLayoutMessage { index_type, earray_params, farray_params, + bt2_params, single_chunk_filter, index_address, }, @@ -921,6 +976,30 @@ mod tests { assert_eq!(decoded, msg); } + /// The BTreeV2 parameters (node size, split/merge) round-trip through + /// the message instead of being skipped on decode and re-stamped with + /// defaults on encode — a rewritten object header must agree with the + /// BTHD it points at. + #[test] + fn roundtrip_chunked_v4_btree_v2_params() { + for ctx in [ctx8(), ctx4()] { + let msg = DataLayoutMessage::chunked_v4_btree_v2( + 4, + vec![2, 2, 8], + Bt2Params { + node_size: 512, + split_percent: 90, + merge_percent: 30, + }, + 0x2000, + ); + let encoded = msg.encode(&ctx); + let (decoded, consumed) = DataLayoutMessage::decode(&encoded, &ctx).unwrap(); + assert_eq!(consumed, encoded.len()); + assert_eq!(decoded, msg); + } + } + /// A filtered single-chunk layout (flag `0x02`) carries the chunk's /// on-disk size and per-chunk filter mask inline. Decode must retain both /// (not discard them), and encode↔decode must round-trip — including the @@ -935,6 +1014,7 @@ mod tests { index_type: ChunkIndexType::SingleChunk, earray_params: None, farray_params: None, + bt2_params: None, single_chunk_filter: Some(SingleChunkFilter { nbytes: 12345, filter_mask: 0b101, diff --git a/src/group.rs b/src/group.rs index f989ac7..79e1b17 100644 --- a/src/group.rs +++ b/src/group.rs @@ -131,12 +131,20 @@ impl H5Group { // This opens empty groups, attribute-only groups, and // subgroup-only groups, which have no datasets beneath them. let inner = borrow_inner(&self.file_inner); - if let H5FileInner::Reader(reader) = &*inner { - let group_path = full_name.trim_start_matches('/'); - if !reader.has_group(group_path) { - return Err(Hdf5Error::NotFound(full_name)); + let full_name = match &*inner { + H5FileInner::Reader(reader) => { + let group_path = full_name.trim_start_matches('/'); + if !reader.has_group(group_path) { + return Err(Hdf5Error::NotFound(full_name)); + } + full_name } - } + // In write mode the handle stores the tree path, so a path + // through hard links resolves once here and every operation + // made through the handle lands on the link's target. + H5FileInner::Writer(writer) => writer.canonical_group_path(&full_name), + H5FileInner::Closed => full_name, + }; drop(inner); Ok(H5Group { @@ -484,12 +492,7 @@ impl H5Group { let inner = borrow_inner(&self.file_inner); match &*inner { H5FileInner::Writer(writer) => { - let attr = writer.vlen_string_attribute(name, value)?; - if self.name == "/" { - writer.add_root_attribute(attr); - } else { - writer.add_group_attribute(&self.name, attr)?; - } + writer.set_vlen_string_attribute(self.attr_target(), name, value)?; Ok(()) } H5FileInner::Reader(_) => Err(Hdf5Error::InvalidState( @@ -595,12 +598,7 @@ impl H5Group { let mut inner = borrow_inner_mut(&self.file_inner); match &mut *inner { H5FileInner::Writer(writer) => { - let attr = writer.vlen_string_array_attribute(name, values, &dims)?; - if self.name == "/" { - writer.add_root_attribute(attr); - } else { - writer.add_group_attribute(&self.name, attr)?; - } + writer.set_vlen_string_array_attribute(self.attr_target(), name, values, &dims)?; Ok(()) } H5FileInner::Reader(_) => Err(Hdf5Error::InvalidState( @@ -610,17 +608,23 @@ impl H5Group { } } + /// The writer-side attribute list this group's attributes live in: the + /// root group's is the file-level list, any other group's is its own. + fn attr_target(&self) -> crate::io::writer::AttrTarget<'_> { + if self.name == "/" { + crate::io::writer::AttrTarget::Root + } else { + crate::io::writer::AttrTarget::Group(&self.name) + } + } + /// Route an attribute to the writer: the root group goes to the /// file-level attribute list, any other group to its own header. fn add_attr(&self, attr: AttributeMessage) -> Result<()> { let inner = borrow_inner(&self.file_inner); match &*inner { H5FileInner::Writer(writer) => { - if self.name == "/" { - writer.add_root_attribute(attr); - } else { - writer.add_group_attribute(&self.name, attr)?; - } + writer.set_attribute(self.attr_target(), attr)?; Ok(()) } H5FileInner::Reader(_) => Err(Hdf5Error::InvalidState( diff --git a/src/io/allocator.rs b/src/io/allocator.rs index 8f3f67b..726e59d 100644 --- a/src/io/allocator.rs +++ b/src/io/allocator.rs @@ -141,6 +141,55 @@ impl FileAllocator { Some(block.addr) } + /// Try to grow the allocation `[addr, addr + len)` by `extra` bytes in + /// place — libhdf5's `H5MF_try_extend`. Returns whether the block now + /// extends to `addr + len + extra`. + /// + /// Two ways it can succeed, tried in `H5MF_try_extend`'s order: the + /// block ends at the end of the file, so the end-of-file pointer moves + /// (published by compare-and-swap, so a concurrent `allocate` cannot be + /// handed the same region); or a released block starts exactly at + /// `addr + len` and is large enough, so the front of it is consumed + /// (`extra` rounded up to the alignment, keeping the remainder aligned). + pub fn try_extend(&self, addr: u64, len: u64, extra: u64) -> bool { + if extra == 0 { + return true; + } + let end = addr + len; + let mut cur = self.eof.load(Ordering::Acquire); + while cur == end { + match self.eof.compare_exchange_weak( + end, + end + extra, + Ordering::AcqRel, + Ordering::Acquire, + ) { + Ok(_) => return true, + Err(actual) => cur = actual, + } + } + + if self.free_count.load(Ordering::Acquire) == 0 { + return false; + } + let mut list = self.free_list.lock().unwrap(); + let used = self.align_up(extra); + let Some(pos) = list.iter().position(|b| b.addr == end && b.len >= used) else { + return false; + }; + let block = list[pos]; + if block.len > used { + list[pos] = FreeBlock { + addr: block.addr + used, + len: block.len - used, + }; + } else { + list.remove(pos); + } + self.free_count.store(list.len() as u64, Ordering::Release); + true + } + /// Return the current end-of-file offset. pub fn eof(&self) -> u64 { self.eof.load(Ordering::Acquire) @@ -330,6 +379,55 @@ mod tests { assert_eq!(alloc.eof(), eof_before); } + #[test] + fn try_extend_grows_the_file_when_the_block_ends_at_eof() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(64); + assert!(alloc.try_extend(a, 64, 32)); + assert_eq!(alloc.eof(), 96); + // The extension owns [64, 96): the next allocation starts after it. + assert_eq!(alloc.allocate(8), 96); + } + + #[test] + fn try_extend_consumes_the_front_of_an_adjacent_free_block() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(64); + let b = alloc.allocate(64); + alloc.allocate(8); // pin: the freed block is not at EOF + alloc.free(b, 64); + + assert!(alloc.try_extend(a, 64, 16)); + assert_eq!(free_blocks(&alloc), vec![(b + 16, 48)]); + // Growing into the whole remainder empties the list. + assert!(alloc.try_extend(a, 80, 48)); + assert!(free_blocks(&alloc).is_empty()); + } + + #[test] + fn try_extend_fails_without_room_past_the_block() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(64); + alloc.allocate(64); // live block right after `a` + let c = alloc.allocate(16); + alloc.free(c, 16); // free space exists, but not at a + 64 + + assert!(!alloc.try_extend(a, 64, 8)); + assert_eq!(free_blocks(&alloc), vec![(c, 16)], "nothing consumed"); + } + + #[test] + fn try_extend_fails_when_the_adjacent_block_is_too_small() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(64); + let b = alloc.allocate(32); + alloc.allocate(8); + alloc.free(b, 32); + + assert!(!alloc.try_extend(a, 64, 40), "32 free < 40 wanted"); + assert_eq!(free_blocks(&alloc), vec![(b, 32)], "nothing consumed"); + } + #[test] fn freeing_nothing_is_a_no_op() { let alloc = FileAllocator::new(0); diff --git a/src/io/reader.rs b/src/io/reader.rs index 2955375..9a238f7 100644 --- a/src/io/reader.rs +++ b/src/io/reader.rs @@ -151,6 +151,12 @@ pub struct Hdf5Reader { /// attributes. Built from actual link records, so empty groups, /// attribute-only groups, and subgroup-only groups are all included. group_paths: std::collections::BTreeSet, + /// Group hard links: alias path → the first-walked path of the same + /// group object header (both without a leading `/`). The walk + /// descends each header once, so objects under the alias are stored + /// under the first path; lookups resolve alias prefixes through this + /// map, as HDF5 path traversal does. + group_aliases: std::collections::HashMap, } /// Total byte length of `dims.product() * element_size`, computed with @@ -401,12 +407,13 @@ impl Hdf5Reader { // Walk link messages to discover datasets, group attributes, and // every group path that exists. - let (datasets, group_attributes, group_paths) = Self::discover_datasets_from_links( - &mut handle, - &root_header, - sb.root_group_object_header_address, - &ctx, - )?; + let (datasets, group_attributes, group_paths, group_aliases) = + Self::discover_datasets_from_links( + &mut handle, + &root_header, + sb.root_group_object_header_address, + &ctx, + )?; // Collect root group attributes let mut root_attributes = Vec::new(); @@ -429,6 +436,7 @@ impl Hdf5Reader { root_attributes, group_attributes, group_paths, + group_aliases, }) } @@ -473,7 +481,7 @@ impl Hdf5Reader { .any(|m| m.msg_type == MSG_LINK || m.msg_type == MSG_LINK_INFO) }); - let (datasets, group_attributes, group_paths) = if has_links { + let (datasets, group_attributes, group_paths, group_aliases) = if has_links { Self::discover_datasets_from_links( &mut handle, root_hdr.as_ref().unwrap(), @@ -506,6 +514,7 @@ impl Hdf5Reader { Vec::new(), std::collections::HashMap::new(), std::collections::BTreeSet::new(), + std::collections::HashMap::new(), ) } }; @@ -523,6 +532,7 @@ impl Hdf5Reader { root_attributes, group_attributes, group_paths, + group_aliases, }) } @@ -552,13 +562,19 @@ impl Hdf5Reader { Vec, std::collections::HashMap>, std::collections::BTreeSet, + std::collections::HashMap, )> { let mut group_attrs = std::collections::HashMap::new(); let mut group_paths = std::collections::BTreeSet::new(); - let mut visited = std::collections::HashSet::new(); + // Object headers already descended into, keyed to the first path + // that reached them: a later path to the same header is a group + // hard link, recorded in `aliases` so lookups can resolve through + // it instead of walking (and cycling) a second time. + let mut visited = std::collections::HashMap::new(); + let mut aliases = std::collections::HashMap::new(); // Seed the root object header so a hard link cycling back to the // root is not descended into a second time. - visited.insert(root_addr); + visited.insert(root_addr, String::new()); let datasets = Self::discover_datasets_recursive( handle, root_header, @@ -567,8 +583,9 @@ impl Hdf5Reader { &mut group_attrs, &mut group_paths, &mut visited, + &mut aliases, )?; - Ok((datasets, group_attrs, group_paths)) + Ok((datasets, group_attrs, group_paths, aliases)) } #[allow(clippy::too_many_arguments)] @@ -579,7 +596,8 @@ impl Hdf5Reader { prefix: &str, group_attrs: &mut std::collections::HashMap>, group_paths: &mut std::collections::BTreeSet, - visited: &mut std::collections::HashSet, + visited: &mut std::collections::HashMap, + aliases: &mut std::collections::HashMap, ) -> IoResult> { // Bound recursion depth on a hostile/corrupt file. const MAX_GROUP_DEPTH: usize = 256; @@ -674,10 +692,14 @@ impl Hdf5Reader { if !attrs.is_empty() { group_attrs.insert(full_name.clone(), attrs); } - // Descend at most once per object header (cycle guard). - if !visited.insert(*address) { + // Descend at most once per object header (cycle + // guard); a second path to it is a group hard + // link — record the alias for lookups instead. + if let Some(first) = visited.get(address) { + aliases.insert(full_name.clone(), first.clone()); continue; } + visited.insert(*address, full_name.clone()); let child_ds = Self::discover_datasets_recursive( handle, &child_header, @@ -686,6 +708,7 @@ impl Hdf5Reader { group_attrs, group_paths, visited, + aliases, )?; datasets.extend(child_ds); } @@ -762,12 +785,16 @@ impl Hdf5Reader { Vec, std::collections::HashMap>, std::collections::BTreeSet, + std::collections::HashMap, )> { let mut datasets = Vec::new(); - let mut visited = std::collections::HashSet::new(); + // First path per descended object header + the group-hard-link + // aliases met later, as in `discover_datasets_from_links`. + let mut visited = std::collections::HashMap::new(); + let mut aliases = std::collections::HashMap::new(); // Seed the root object header so a hard link cycling back to the // root group is not descended into a second time. - visited.insert(root_obj_addr); + visited.insert(root_obj_addr, String::new()); let mut group_attrs = std::collections::HashMap::new(); let mut group_paths = std::collections::BTreeSet::new(); Self::discover_datasets_from_btree_recursive( @@ -779,10 +806,11 @@ impl Hdf5Reader { 0, &mut datasets, &mut visited, + &mut aliases, &mut group_attrs, &mut group_paths, )?; - Ok((datasets, group_attrs, group_paths)) + Ok((datasets, group_attrs, group_paths, aliases)) } /// Recursive worker for `discover_datasets_from_btree`. `prefix` is the @@ -796,7 +824,8 @@ impl Hdf5Reader { prefix: &str, depth: usize, datasets: &mut Vec, - visited: &mut std::collections::HashSet, + visited: &mut std::collections::HashMap, + aliases: &mut std::collections::HashMap, group_attrs: &mut std::collections::HashMap>, group_paths: &mut std::collections::BTreeSet, ) -> IoResult<()> { @@ -878,10 +907,13 @@ impl Hdf5Reader { group_paths.insert(full_name.clone()); // Break cycles: descend into each group object header at - // most once. - if !visited.insert(entry.obj_header_addr) { + // most once. A second path to it is a group hard link — + // record the alias for lookups instead. + if let Some(first) = visited.get(&entry.obj_header_addr) { + aliases.insert(full_name.clone(), first.clone()); continue; } + visited.insert(entry.obj_header_addr, full_name.clone()); // Collect this subgroup's attributes (e.g. NeXus NX_class). { @@ -919,6 +951,7 @@ impl Hdf5Reader { depth + 1, datasets, visited, + aliases, group_attrs, group_paths, )?; @@ -1192,8 +1225,36 @@ impl Hdf5Reader { self.datasets.iter().map(|d| d.name.as_str()).collect() } - /// Return metadata for a dataset by name. + /// Rewrite a path (no leading `/`) whose group components pass + /// through hard links into the first-walked path of the object it + /// reaches — HDF5 traversal over the aliases the discovery walk + /// recorded. Bounded like libhdf5's link-traversal limit, so a link + /// cycle cannot loop forever; a path with no alias components comes + /// back unchanged. + fn canonical_path(&self, name: &str) -> String { + let mut name = name.to_string(); + for _ in 0..64 { + let mut best: Option<(&str, &str)> = None; + for (alias, first) in &self.group_aliases { + let covers = name == *alias || name.starts_with(&format!("{alias}/")); + if covers && best.is_none_or(|(a, _)| alias.len() > a.len()) { + best = Some((alias, first)); + } + } + let Some((alias, first)) = best else { break }; + // `first` is empty for an alias of the root group; trimming + // keeps the no-leading-'/' form either way. + name = format!("{first}{}", &name[alias.len()..]) + .trim_start_matches('/') + .to_string(); + } + name + } + + /// Return metadata for a dataset by name. Like `H5Dopen`, the name + /// may pass through group hard links. pub fn dataset_info(&self, name: &str) -> Option<&DatasetReadInfo> { + let name = self.canonical_path(name); self.datasets.iter().find(|d| d.name == name) } @@ -1230,10 +1291,11 @@ impl Hdf5Reader { } /// Return the attribute names of a non-root group (path without a - /// leading `/`, e.g. `"detector"` or `"entry/instrument"`). + /// leading `/`, e.g. `"detector"` or `"entry/instrument"`; may pass + /// through group hard links). pub fn group_attr_names(&self, group_path: &str) -> Vec { self.group_attributes - .get(group_path) + .get(&self.canonical_path(group_path)) .map(|v| v.iter().map(|a| a.name.clone()).collect()) .unwrap_or_default() } @@ -1241,7 +1303,7 @@ impl Hdf5Reader { /// Return a non-root group's attribute by name. pub fn group_attr(&self, group_path: &str, name: &str) -> Option<&AttributeMessage> { self.group_attributes - .get(group_path)? + .get(&self.canonical_path(group_path))? .iter() .find(|a| a.name == name) } @@ -1253,10 +1315,42 @@ impl Hdf5Reader { &self.group_paths } - /// Report whether a group exists at `group_path` (no leading `/`). - /// The empty string denotes the root group, which always exists. + /// Report whether a group exists at `group_path` (no leading `/`; + /// may pass through group hard links). The empty string denotes the + /// root group, which always exists. pub fn has_group(&self, group_path: &str) -> bool { - group_path.is_empty() || self.group_paths.contains(group_path) + if group_path.is_empty() || self.group_paths.contains(group_path) { + return true; + } + let canon = self.canonical_path(group_path); + canon.is_empty() || self.group_paths.contains(&canon) + } + + /// Read and decode the global-heap collection at `addr`, applying the + /// validation of libhdf5's `H5HG__cache_heap_deserialize`: the `GCOL` + /// signature must be present and the declared size at least + /// `H5HG_MINSIZE` (4096 bytes). There is no upper size cap — libhdf5 + /// has none, and this crate's writers put a whole write call's strings + /// into one collection, which a cap would turn into silent data loss. + fn read_heap_collection(&mut self, addr: u64) -> IoResult { + let ss = self.ctx.sizeof_size as usize; + let header_len = 4 + 1 + 3 + ss; + let header_buf = self.handle.read_at_most(addr, header_len)?; + if header_buf.len() < header_len || header_buf[0..4] != *b"GCOL" { + return Err(crate::io::IoError::InvalidState(format!( + "bad global heap collection signature at address {addr:#x}" + ))); + } + let declared = read_uint(&header_buf[8..], ss) as usize; + if declared < 4096 { + return Err(crate::io::IoError::InvalidState(format!( + "global heap collection at address {addr:#x} declares size {declared}, \ + below the 4096-byte minimum" + ))); + } + let heap_buf = self.handle.read_at(addr, declared)?; + let (coll, _) = GlobalHeapCollection::decode(&heap_buf, &self.ctx)?; + Ok(coll) } /// Decode an attribute's value as a string, resolving a variable-length @@ -1282,19 +1376,17 @@ impl Hdf5Reader { if coll_addr == UNDEF_ADDR || coll_addr == 0 { return Ok(String::new()); } - let ss = self.ctx.sizeof_size as usize; - let header_len = 4 + 1 + 3 + ss; - let header_buf = self.handle.read_at_most(coll_addr, header_len)?; - if header_buf.len() < header_len || header_buf[0..4] != *b"GCOL" { - return Ok(String::new()); - } - let collection_size = read_uint(&header_buf[8..], ss) as usize; - if collection_size == 0 || collection_size > 64 * 1024 * 1024 { - return Ok(String::new()); - } - let heap_buf = self.handle.read_at(coll_addr, collection_size)?; - let (coll, _) = GlobalHeapCollection::decode(&heap_buf, &self.ctx)?; - let obj = coll.get_object(obj_index as u16).unwrap_or(&[]); + let coll = self.read_heap_collection(coll_addr)?; + let idx = u16::try_from(obj_index).map_err(|_| { + crate::io::IoError::InvalidState(format!( + "global heap object index {obj_index} does not fit the 16-bit on-disk field" + )) + })?; + let obj = coll.get_object(idx).ok_or_else(|| { + crate::io::IoError::InvalidState(format!( + "global heap object {idx} not found in the collection at address {coll_addr:#x}" + )) + })?; Ok(String::from_utf8_lossy(obj).to_string()) } @@ -1475,18 +1567,20 @@ impl Hdf5Reader { // Re-scan datasets, group attributes, and group paths from link // messages. - let (datasets, group_attributes, group_paths) = Self::discover_datasets_from_links( - &mut self.handle, - &root_header, - sb.root_group_object_header_address, - &ctx, - )?; + let (datasets, group_attributes, group_paths, group_aliases) = + Self::discover_datasets_from_links( + &mut self.handle, + &root_header, + sb.root_group_object_header_address, + &ctx, + )?; self._eof = sb.end_of_file_address; self.ctx = ctx; self.datasets = datasets; self.group_attributes = group_attributes; self.group_paths = group_paths; + self.group_aliases = group_aliases; Ok(()) } @@ -2593,22 +2687,7 @@ impl Hdf5Reader { // Read or get cached global heap collection #[allow(clippy::map_entry)] if !heap_cache.contains_key(&collection_addr) { - let ss = self.ctx.sizeof_size as usize; - let header_len = 4 + 1 + 3 + ss; - let header_buf = self.handle.read_at_most(collection_addr, header_len)?; - if header_buf.len() < header_len || &header_buf[0..4] != b"GCOL" { - items.push(Vec::new()); - continue; - } - let collection_size = read_uint(&header_buf[8..], ss) as usize; - - if collection_size == 0 || collection_size > 64 * 1024 * 1024 { - items.push(Vec::new()); - continue; - } - - let heap_buf = self.handle.read_at(collection_addr, collection_size)?; - let (coll, _) = GlobalHeapCollection::decode(&heap_buf, &self.ctx)?; + let coll = self.read_heap_collection(collection_addr)?; let lookup: std::collections::HashMap = coll .objects .iter() @@ -2618,12 +2697,20 @@ impl Hdf5Reader { heap_cache.insert(collection_addr, (coll, lookup)); } + let idx = u16::try_from(obj_index).map_err(|_| { + crate::io::IoError::InvalidState(format!( + "global heap object index {obj_index} does not fit the 16-bit on-disk field \ + (element {i} of \"{name}\")" + )) + })?; let (coll, lookup) = &heap_cache[&collection_addr]; - if let Some(&idx) = lookup.get(&(obj_index as u16)) { - items.push(coll.objects[idx].data.clone()); - } else { - items.push(Vec::new()); - } + let &oi = lookup.get(&idx).ok_or_else(|| { + crate::io::IoError::InvalidState(format!( + "global heap object {idx} not found in the collection at address \ + {collection_addr:#x} (element {i} of \"{name}\")" + )) + })?; + items.push(coll.objects[oi].data.clone()); } Ok(items) diff --git a/src/io/writer.rs b/src/io/writer.rs index 3d9a2e9..6fa4ab6 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -13,8 +13,9 @@ use crate::format::chunk_index::extensible_array::{ EA_CLS_CHUNK, EA_CLS_FILT_CHUNK, }; use crate::format::chunk_index::fixed_array::{ - encode_filtered_page, encode_unfiltered_page, FixedArrayDataBlock, - FixedArrayFilteredChunkElement, FixedArrayHeader, FixedArrayPagedPrefix, FA_CLIENT_FILT_CHUNK, + decode_filtered_page, decode_unfiltered_page, encode_filtered_page, encode_unfiltered_page, + FixedArrayDataBlock, FixedArrayFilteredChunkElement, FixedArrayHeader, FixedArrayPagedPrefix, + FA_CLIENT_FILT_CHUNK, }; use crate::format::messages::attribute::AttributeMessage; use crate::format::messages::data_layout::{DataLayoutMessage, EarrayParams, FixedArrayParams}; @@ -64,6 +65,66 @@ fn fixed_array_dblk_disk_size(ctx: &FormatContext, hdr: &FixedArrayHeader) -> u6 } } +/// Walk a v2 B-tree from `addr`, collecting every node's raw record bytes +/// and every node block's address — the reader's record walk plus the +/// addresses, which `open_append` needs so the reconstructed +/// [`Bt2DatasetInfo::node_addrs`] pool owns the on-disk nodes (the next +/// flush re-serializes the tree over them, and a delete frees them). +#[allow(clippy::too_many_arguments)] +fn collect_bt2_nodes( + handle: &FileHandle, + ctx: &FormatContext, + addr: u64, + depth: u16, + nrec: u16, + record_size: u16, + node_size: u32, + geo: &crate::format::chunk_index::btree_v2::Bt2Geometry, + records: &mut Vec, + node_addrs: &mut Vec, +) -> IoResult<()> { + use crate::format::chunk_index::btree_v2::{Bt2InternalNode, Bt2LeafNode}; + + node_addrs.push(addr); + let buf = handle.read_at_most(addr, node_size as usize)?; + if depth == 0 { + let leaf = Bt2LeafNode::decode(&buf, nrec, record_size)?; + records.extend_from_slice(&leaf.record_data); + } else { + let node = Bt2InternalNode::decode( + &buf, + ctx, + depth, + nrec, + record_size, + geo.max_nrec_size, + geo.child_total_size(depth), + )?; + records.extend_from_slice(&node.record_data); + let children: Vec<(u64, u16)> = node + .child_addrs + .iter() + .zip(node.child_nrecords.iter()) + .map(|(&a, &n)| (a, n)) + .collect(); + for (child_addr, child_nrec) in children { + collect_bt2_nodes( + handle, + ctx, + child_addr, + depth - 1, + child_nrec, + record_size, + node_size, + geo, + records, + node_addrs, + )?; + } + } + Ok(()) +} + /// Encode a fixed-array data block for the layout implied by `hdr`, using the /// chunk addresses held in `dblk.elements` (unfiltered) or the filtered chunk /// entries in `dblk.filtered_elements` (filtered, `client_id == 1`). @@ -147,6 +208,64 @@ fn encode_fixed_array_dblk( buf } +/// Decode a fixed-array data block for the layout implied by `hdr` — the +/// inverse of [`encode_fixed_array_dblk`], and the single decode dispatch +/// over non-paged/paged × unfiltered/filtered. +/// +/// For the paged layout, pages whose bitmap bit is clear are skipped, not +/// decoded: libhdf5 never writes an uninitialized page, so its bytes are +/// arbitrary and carry no valid checksum. Their elements stay at the +/// undefined-address defaults, which is exactly what the bitmap means. +fn decode_fixed_array_dblk( + ctx: &FormatContext, + hdr: &FixedArrayHeader, + buf: &[u8], + chunk_size_len: usize, +) -> crate::format::FormatResult { + let is_filtered = hdr.client_id == FA_CLIENT_FILT_CHUNK; + let num_elmts = hdr.num_elmts as usize; + + if !hdr.is_paged() { + return if is_filtered { + FixedArrayDataBlock::decode_filtered(buf, ctx, num_elmts, chunk_size_len) + } else { + FixedArrayDataBlock::decode_unfiltered(buf, ctx, num_elmts) + }; + } + + let npages = hdr.npages() as usize; + let dblk_page_nelmts = hdr.dblk_page_nelmts() as usize; + let prefix = FixedArrayPagedPrefix::decode(buf, ctx, npages as u64)?; + + let mut dblk = if is_filtered { + FixedArrayDataBlock::new_filtered(prefix.header_addr, num_elmts) + } else { + FixedArrayDataBlock::new_unfiltered(prefix.header_addr, num_elmts) + }; + dblk.client_id = hdr.client_id; + + // Pages follow the prefix back to back; every page spans the full + // `dblk_page_nelmts` stride except the last, which holds the remainder. + let mut pos = prefix.prefix_size; + for p in 0..npages { + let start = p * dblk_page_nelmts; + let end = ((p + 1) * dblk_page_nelmts).min(num_elmts); + let nelmts = end - start; + if prefix.page_initialized(p) { + let page_buf = buf.get(pos..).unwrap_or(&[]); + if is_filtered { + let elems = decode_filtered_page(page_buf, ctx, nelmts, chunk_size_len)?; + dblk.filtered_elements[start..end].clone_from_slice(&elems); + } else { + let addrs = decode_unfiltered_page(page_buf, ctx, nelmts)?; + dblk.elements[start..end].copy_from_slice(&addrs); + } + } + pos += nelmts * hdr.element_size as usize + 4; + } + Ok(dblk) +} + /// Interior-mutability cell for per-dataset write state, selected by feature. /// /// This is the §5-B "cfg-selected interior types" from @@ -187,12 +306,16 @@ impl Slot { /// Proof that the create gate (`create_lock`) is held and the new dataset's /// name passed the uniqueness check. Only [`Hdf5Writer::begin_create`] /// constructs one and [`Hdf5Writer::push_dataset`] demands one, so a creator -/// cannot reach the dataset registry while skipping either step. +/// cannot reach the dataset registry while skipping either step. Carries +/// the canonical (link-resolved) name the creator must store, so the +/// registry only ever holds tree paths. pub(crate) struct CreateGuard<'a> { #[cfg(not(feature = "threadsafe"))] _gate: std::cell::RefMut<'a, ()>, #[cfg(feature = "threadsafe")] _gate: std::sync::MutexGuard<'a, ()>, + /// The dataset name with every group hard link in it resolved. + pub(crate) name: String, } /// Reference-counted shared pointer, feature-selected. The single-thread @@ -322,6 +445,12 @@ pub struct DatasetInfo { pub filter_pipeline: Option, /// Soft-deleted: excluded from finalize output. pub deleted: bool, + /// The dataspace extent changed this session (`extend_dataset` / + /// `set_dataset_extent`). On a reopened dataset the finalize gate + /// otherwise infers "modified" from `chunks_written` alone, and a + /// session that only changed the extent would keep the old on-disk + /// header — silently dropping the new shape. + pub extent_dirty: bool, /// User-defined fill value bytes (exactly one element wide). `None` /// means default zero-fill; `Some` is emitted as a `fill_defined = 2` /// fill-value message in the dataset object header. @@ -372,6 +501,18 @@ enum DblkParent { }, } +/// Which attribute list an attribute operation targets: the root group's, +/// a group's (by full path), or a dataset's (by writer index). +#[derive(Clone, Copy)] +pub enum AttrTarget<'a> { + /// The root group's (file-level) attributes. + Root, + /// A group's attributes, by full path. + Group(&'a str), + /// A dataset's attributes, by writer index. + Dataset(usize), +} + /// Which chunk index a dataset uses. libhdf5 picks it from the dataspace: a /// v2 B-tree for two or more unlimited dimensions, an extensible array for /// exactly one, a fixed array for none. @@ -414,6 +555,136 @@ impl ChunkGeometry { } } +/// The refusal every attribute mutation gets while SWMR streaming is +/// active, from the two owners of attribute-list change +/// ([`Hdf5Writer::set_attribute`] and `evict_attr`). +fn swmr_attr_error(name: &str) -> crate::io::IoError { + crate::io::IoError::InvalidState(format!( + "cannot add or modify attribute '{name}' during SWMR streaming: object \ + headers are frozen while readers stream, and a superseded variable-length \ + value's heap storage could never be reclaimed; set attributes before \ + start_swmr (libhdf5 forbids attribute changes during SWMR writes too)" + )) +} + +/// One collection block with free space that a later vlen insert may +/// fill — an entry in the writer's CWFS list (libhdf5 `f->shared->cwfs`). +struct CwfsEntry { + /// Block address of the collection. + addr: u64, + /// Declared block size; never changes after allocation. + size: usize, + /// Bytes its free-space marker owns, per + /// [`GlobalHeapCollection::free_space_at`](crate::format::global_heap::GlobalHeapCollection::free_space_at). + free: usize, +} + +/// Maximum CWFS entries tracked — libhdf5's `H5HG_NCWFS` (H5HGpkg.h). +const H5HG_NCWFS: usize = 16; + +/// Record a collection with `free` bytes in the CWFS list: update its +/// entry if present, append while the list is short, and otherwise +/// replace the entry with the least free space when this one has more — +/// the retention rule of libhdf5's `H5HG_insert`. +fn cwfs_note(cwfs: &mut Vec, addr: u64, size: usize, free: usize) { + if let Some(p) = cwfs.iter().position(|e| e.addr == addr) { + cwfs[p].free = free; + return; + } + if cwfs.len() < H5HG_NCWFS { + cwfs.insert(0, CwfsEntry { addr, size, free }); + return; + } + if let Some(p) = (0..cwfs.len()).min_by_key(|&p| cwfs[p].free) { + if free > cwfs[p].free { + cwfs[p] = CwfsEntry { addr, size, free }; + } + } +} + +/// The uniform rejection for `delete_dataset` / `delete_group` while SWMR +/// streaming is active: deleting frees the object's blocks, and a live +/// reader may hold any of their addresses. +fn swmr_delete_error(name: &str) -> crate::io::IoError { + crate::io::IoError::InvalidState(format!( + "cannot delete '{name}' during SWMR streaming: a reader may hold the \ + object's header and storage addresses (libhdf5 forbids link deletion \ + during SWMR writes too)" + )) +} + +/// Whether the chunk at grid `coords` lies entirely at or beyond `extent` in +/// some dimension — no element of it would survive a shrink to that extent. +fn chunk_outside_extent(coords: &[u64], chunk_dims: &[u64], extent: &[u64]) -> bool { + coords + .iter() + .zip(chunk_dims) + .zip(extent) + .any(|((&c, &cd), &e)| c.saturating_mul(cd) >= e) +} + +/// Whether the chunk at grid `coords` keeps elements under `extent` but +/// extends past it in some dimension — a shrink must refill its +/// out-of-extent region with the fill value. +fn chunk_straddles_extent(coords: &[u64], chunk_dims: &[u64], extent: &[u64]) -> bool { + !chunk_outside_extent(coords, chunk_dims, extent) + && coords + .iter() + .zip(chunk_dims) + .zip(extent) + .any(|((&c, &cd), &e)| (c + 1).saturating_mul(cd) > e) +} + +/// Overwrite, in `data` (one whole chunk, unfiltered, row-major), every +/// element at or beyond `extent` with the matching bytes of `fill` — a +/// same-sized buffer tiled with the fill value. The caller guarantees the +/// chunk at `coords` straddles `extent`, so every dimension keeps at least +/// one element. Returns the replaced bytes, so a vlen dataset's dead +/// heap references can be released rather than stranded. +fn refill_chunk_beyond_extent( + data: &mut [u8], + fill: &[u8], + coords: &[u64], + chunk_dims: &[u64], + extent: &[u64], + element_size: usize, +) -> Vec { + let ndims = chunk_dims.len(); + let keep: Vec = (0..ndims) + .map(|d| { + let origin = coords[d] * chunk_dims[d]; + chunk_dims[d].min(extent[d].saturating_sub(origin)) as usize + }) + .collect(); + // Row-major walk: for every row (all dimensions but the last), + // overwrite the whole row when its prefix is outside the keep box, + // else only the row's out-of-extent tail. + let row_elems = chunk_dims[ndims - 1] as usize; + let keep_last = keep[ndims - 1]; + let nrows: u64 = chunk_dims[..ndims - 1].iter().product(); + let mut replaced = Vec::new(); + for r in 0..nrows { + let mut rem = r; + let mut in_keep = true; + for d in (0..ndims - 1).rev() { + let c = rem % chunk_dims[d]; + rem /= chunk_dims[d]; + if c as usize >= keep[d] { + in_keep = false; + } + } + let start = if in_keep { keep_last } else { 0 }; + if start == row_elems { + continue; + } + let a = (r as usize * row_elems + start) * element_size; + let b = (r as usize + 1) * row_elems * element_size; + replaced.extend_from_slice(&data[a..b]); + data[a..b].copy_from_slice(&fill[a..b]); + } + replaced +} + /// Validate caller-supplied chunk geometry at dataset definition, the rule /// libhdf5 applies in `H5D__chunk_construct` (H5Dchunk.c): the chunk rank /// must match the dataspace rank, no chunk dimension may be zero, and a @@ -510,8 +781,9 @@ pub struct Bt2DatasetInfo { pub max_dims: Vec, /// File offset of the BT2 header. pub bt2_header_addr: u64, - /// Pool of `BT2_NODE_SIZE`-byte blocks holding the tree's nodes, in the - /// order [`Bt2Tree::encode`] emits them. + /// Pool of node-size blocks (the index's + /// [`node_size`](Bt2ChunkIndex::node_size) bytes each) holding the tree's + /// nodes, in the order [`Bt2Tree::encode`] emits them. /// /// The single owner of the tree's node addresses: a flush re-serializes the /// whole tree over these blocks and allocates only the shortfall, so no @@ -542,6 +814,11 @@ pub struct GroupInfo { pub child_groups: Vec, /// File offset of this group's object header (set during finalize). pub obj_header_addr: u64, + /// File offset of the on-disk header a reopen found for this group, so + /// finalize can free the block it supersedes. + pub obj_header_written_addr: Option, + /// Encoded size of that on-disk header (first block). + pub obj_header_encoded_size: usize, /// Soft-deleted: excluded from finalize output. pub deleted: bool, /// Attributes attached to this group (e.g. NeXus `NX_class`). @@ -637,10 +914,25 @@ pub struct Hdf5Writer { /// libhdf5's `H5D__chunk_file_alloc`, which skips `H5MF_xfree` under /// `H5F_ACC_SWMR_WRITE`. swmr_active: bool, + /// Collections with free space — libhdf5's `f->shared->cwfs` list. A + /// vlen insert fills these partially-filled collection blocks before + /// creating a new one, so many small writes share 4096-byte blocks + /// instead of each taking their own. Entries hold `(addr, block size, + /// free bytes)` hints; the block on disk stays the single truth for + /// contents, and only the two functions that rewrite collection blocks + /// ([`insert_vlen_objects`](Self::insert_vlen_objects) and + /// [`release_vlen_references`](Self::release_vlen_references)) may + /// update this list. In-memory only, like the allocator's free list: + /// a reopened file's free space is rediscovered as releases touch its + /// collections. Capped at [`H5HG_NCWFS`] entries. + cwfs: Slot>, /// Address of the root group object header (set after first finalize). root_group_addr: Option, /// Size of the encoded root group object header (for in-place rewrites). root_group_encoded_size: usize, + /// The on-disk root header block a reopen found, `(addr, len)`, so + /// finalize can free the block its rewrite supersedes. + superseded_root_header: Option<(u64, u64)>, } impl Hdf5Writer { @@ -692,8 +984,10 @@ impl Hdf5Writer { libver_latest: false, closed: false, swmr_active: false, + cwfs: Slot::new(Vec::new()), root_group_addr: None, root_group_encoded_size: 0, + superseded_root_header: None, }) } @@ -773,8 +1067,13 @@ impl Hdf5Writer { /// (see `create_lock`) at every creator by construction. pub(crate) fn begin_create(&self, name: &str) -> IoResult> { let gate = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; - Ok(CreateGuard { _gate: gate }) + // A creation path through hard links lands in the link's target + // group, as HDF5 traversal does. Canonicalizing here — the one + // entry every creator passes — keeps alias forms out of the + // registry. + let name = self.canonical_dataset_path(name); + self.ensure_unique_dataset_name(&name)?; + Ok(CreateGuard { _gate: gate, name }) } /// Push a freshly-built dataset into the registry and return its index. @@ -875,7 +1174,8 @@ impl Hdf5Writer { let root_addr = sb.root_group_object_header_address; let root_buf = handle.read_at_most(root_addr, file_size.saturating_sub(root_addr) as usize)?; - let (root_header, _) = crate::format::object_header::ObjectHeader::decode(&root_buf)?; + let (root_header, root_header_size) = + crate::format::object_header::ObjectHeader::decode(&root_buf)?; // Collect existing root-level attributes let mut root_attributes = Vec::new(); @@ -901,12 +1201,36 @@ impl Hdf5Writer { 0, )?; + // Two link entries can share one object header — hard links. Only + // the first-walked path becomes the object; the rest are rebuilt + // as hard-link registry entries further down. Without this split + // every alias came back as its own DatasetInfo carrying the same + // storage addresses, so deleting (or finalizing) one freed blocks + // the others still referenced. + let mut seen_header_addrs = std::collections::HashSet::new(); + let mut alias_entries: Vec<(String, u64)> = Vec::new(); + link_entries.retain(|(name, addr)| { + if seen_header_addrs.insert(*addr) { + true + } else { + alias_entries.push((name.clone(), *addr)); + false + } + }); + let mut existing_datasets = Vec::new(); + // Non-dataset link targets (groups): header block `(addr, len)` by + // link path — so finalize can free the block its rewrite supersedes — + // plus the attributes the header carries, which the group registry + // below must keep or finalize rewrites the group without them. + type GroupHeaderInfo = (u64, usize, Vec); + let mut group_headers: std::collections::HashMap = + Default::default(); for (name, obj_addr) in &link_entries { // Read the dataset's full object header (to EOF — see above). let ds_buf = handle.read_at_most(*obj_addr, file_size.saturating_sub(*obj_addr) as usize)?; - let (ds_header, _) = + let (ds_header, ds_header_size) = match crate::format::object_header::ObjectHeader::decode_any(&ds_buf) { Ok(h) => h, Err(_) => continue, @@ -961,7 +1285,13 @@ impl Hdf5Writer { let (dt, ds, dl) = match (datatype, dataspace, layout) { (Some(dt), Some(ds), Some(dl)) => (dt, ds, dl), - _ => continue, // Not a dataset (probably a group) + _ => { + // Not a dataset — a group's header. Remember its block so + // finalize can free what its rewrite supersedes, and its + // attributes so the registry rebuild keeps them. + group_headers.insert(name.clone(), (*obj_addr, ds_header_size, attrs)); + continue; + } }; let mut info = DatasetInfo { @@ -977,9 +1307,10 @@ impl Hdf5Writer { append: None, attributes: attrs, obj_header_written_addr: Some(*obj_addr), - obj_header_encoded_size: 0, + obj_header_encoded_size: ds_header_size, filter_pipeline: fp, deleted: false, + extent_dirty: false, fill_value, // Preserve the on-disk layout version so finalize re-encodes // what it read: a v5 file reopened and appended to must not be @@ -1126,8 +1457,170 @@ impl Hdf5Writer { chunk_size_len, }); } + } else if *index_type + == crate::format::messages::data_layout::ChunkIndexType::FixedArray + { + // Read the FA header and data block back so a + // reopened dataset is writable and deletable, not + // re-link only — a placeholder made a delete free + // just the header and leak every chunk plus the + // index. Paged data blocks (any FA with more than + // dblk_page_nelmts chunks, libhdf5 default 1024) + // reconstruct through the same decode owner; only + // pages the bitmap marks initialized are decoded. + let hdr_buf = handle.read_at_most(*index_address, 256)?; + let fa_header = FixedArrayHeader::decode(&hdr_buf, &ctx)?; + let is_filtered = fa_header.client_id == FA_CLIENT_FILT_CHUNK; + let chunk_size_len = if is_filtered { + (fa_header.element_size as usize) + .checked_sub(ctx.sizeof_addr as usize + 4) + .ok_or_else(|| { + crate::io::IoError::InvalidState( + "fixed array filtered element_size too small".into(), + ) + })? + } else { + 0 + }; + if fa_header.data_blk_addr != UNDEF_ADDR && chunk_size_len <= 8 { + let dblk_size = fixed_array_dblk_disk_size(&ctx, &fa_header) as usize; + let dblk_buf = + handle.read_at_most(fa_header.data_blk_addr, dblk_size)?; + let fa_dblk = decode_fixed_array_dblk( + &ctx, + &fa_header, + &dblk_buf, + chunk_size_len, + )?; + info.fixed_array = Some(FixedArrayDatasetInfo { + chunk_dims: real_chunk_dims, + fa_header_addr: *index_address, + fa_dblk_addr: fa_header.data_blk_addr, + fa_header, + fa_dblk, + // Chunks written this session, matching the + // EA reconstruction above. + chunks_written: 0, + }); + } + } else if *index_type + == crate::format::messages::data_layout::ChunkIndexType::BTreeV2 + { + use crate::format::chunk_index::btree_v2::{ + Bt2Geometry, Bt2Header, BT2_TYPE_CHUNK_FILT, BT2_TYPE_CHUNK_UNFILT, + }; + + // Walk the tree back into the in-memory index and + // adopt its node blocks as the flush pool. The pool + // re-serializes at the header's node_size, whatever + // it is — libhdf5 sizes every node from + // hdr->node_size (H5B2leaf.c, H5B2internal.c) — so + // a foreign size reopens too. Only a record type + // that is not a chunk record, or a node size below + // the bulk loader's few-records-per-node floor + // (the same bound creation enforces), stays + // re-link only. + let hdr_buf = handle.read_at_most(*index_address, 256)?; + let bt2_hdr = Bt2Header::decode(&hdr_buf, &ctx)?; + let ndims = real_chunk_dims.len(); + let is_filt = match bt2_hdr.record_type { + BT2_TYPE_CHUNK_UNFILT => Some(false), + BT2_TYPE_CHUNK_FILT => Some(true), + _ => None, + }; + if let (Some(is_filt), true) = ( + is_filt, + bt2_hdr.node_size as usize >= 10 + 3 * bt2_hdr.record_size as usize, + ) { + let mut index = if is_filt { + let csl = (bt2_hdr.record_size as usize) + .checked_sub(ctx.sizeof_addr as usize + 4 + ndims * 8) + .filter(|&c| c <= 8) + .ok_or_else(|| { + crate::io::IoError::InvalidState( + "v2 B-tree filtered record size does not fit \ + its rank and address width" + .into(), + ) + })?; + Bt2ChunkIndex::new_filtered(ndims, csl as u8) + } else { + Bt2ChunkIndex::new_unfiltered(ndims) + }; + // Re-serialize with the creator's parameters: + // node blocks keep their size and the rewritten + // header keeps its declared split/merge. + index.node_size = bt2_hdr.node_size; + index.split_percent = bt2_hdr.split_percent; + index.merge_percent = bt2_hdr.merge_percent; + let mut node_addrs = Vec::new(); + if bt2_hdr.root_node_addr != UNDEF_ADDR && bt2_hdr.total_num_records > 0 + { + let geo = Bt2Geometry::new( + bt2_hdr.node_size, + bt2_hdr.record_size, + bt2_hdr.depth, + ctx.sizeof_addr, + ); + let mut record_bytes = Vec::new(); + collect_bt2_nodes( + &handle, + &ctx, + bt2_hdr.root_node_addr, + bt2_hdr.depth, + bt2_hdr.num_records_in_root, + bt2_hdr.record_size, + bt2_hdr.node_size, + &geo, + &mut record_bytes, + &mut node_addrs, + )?; + let total = if bt2_hdr.record_size > 0 { + record_bytes.len() / bt2_hdr.record_size as usize + } else { + 0 + }; + if is_filt { + for r in Bt2ChunkIndex::decode_filtered_records( + &record_bytes, + total, + ndims, + bt2_hdr.record_size, + &ctx, + )? { + index.insert_filtered( + r.scaled_offsets, + r.chunk_address, + r.chunk_size, + r.filter_mask, + ); + } + } else { + for r in Bt2ChunkIndex::decode_unfiltered_records( + &record_bytes, + total, + ndims, + &ctx, + )? { + index.insert(r.scaled_offsets, r.chunk_address); + } + } + } + let max_dims = info + .dataspace + .max_dims + .clone() + .unwrap_or_else(|| info.dataspace.dims.clone()); + info.btree_v2 = Some(Bt2DatasetInfo { + chunk_dims: real_chunk_dims, + max_dims, + bt2_header_addr: *index_address, + node_addrs, + index, + chunks_written: 0, + }); + } } - // FA/BT2 datasets remain as placeholder (re-link only) } _ => {} } @@ -1135,20 +1628,26 @@ impl Hdf5Writer { existing_datasets.push(info); } - // Reconstruct group structure from dataset paths. - // e.g. dataset "nodes/id" implies group "/nodes" exists. + // Reconstruct the group registry. Every group is a link entry of its + // own, whether or not a dataset lives under it, so the registry is + // built from the discovered links — rebuilding it from dataset paths + // alone made attribute-only and empty groups vanish at close, and + // dropped the attributes of the groups that survived. let mut groups: Vec = Vec::new(); let mut group_index_map: std::collections::HashMap = std::collections::HashMap::new(); - for (di, ds) in existing_datasets.iter().enumerate() { - let parts: Vec<&str> = ds.name.split('/').collect(); - if parts.len() <= 1 { - continue; // root-level dataset, no group - } - // Build group hierarchy: e.g. "a/b/c" → groups "/a", "/a/b" + // Register the chain of groups "/a", "/a/b", … for the link-style + // path `link_path` ("a/b"), taking each one's on-disk header block + // and attributes out of `group_headers` when the link walk saw it. + fn ensure_groups_for( + link_path: &str, + groups: &mut Vec, + group_index_map: &mut std::collections::HashMap, + group_headers: &mut std::collections::HashMap, + ) { let mut path = String::new(); - for part in &parts[..parts.len() - 1] { + for part in link_path.split('/') { let parent_path = if path.is_empty() { "/".to_string() } else { @@ -1168,29 +1667,88 @@ impl Hdf5Writer { group_index_map.get(&parent_path).copied() }; let gidx = groups.len(); + let (obj_header_written_addr, obj_header_encoded_size, attributes) = group_headers + .remove(path.trim_start_matches('/')) + .map_or((None, 0, Vec::new()), |(addr, len, attrs)| { + (Some(addr), len, attrs) + }); groups.push(GroupInfo { name: path.clone(), parent, child_datasets: Vec::new(), child_groups: Vec::new(), obj_header_addr: 0, + obj_header_written_addr, + obj_header_encoded_size, deleted: false, - attributes: Vec::new(), + attributes, }); if let Some(pidx) = parent { groups[pidx].child_groups.push(gidx); } group_index_map.insert(path.clone(), gidx); } - // Assign dataset to its immediate parent group - let parent_path = if parts.len() == 2 { - format!("/{}", parts[0]) + } + + // Every linked group, in link-walk order (parents precede children). + for (name, _) in &link_entries { + if group_headers.contains_key(name.as_str()) { + ensure_groups_for(name, &mut groups, &mut group_index_map, &mut group_headers); + } + } + + // Assign each dataset to its immediate parent group, creating any + // group the link walk could not decode (its chain stays placeholder). + for (di, ds) in existing_datasets.iter().enumerate() { + let parts: Vec<&str> = ds.name.split('/').collect(); + if parts.len() <= 1 { + continue; // root-level dataset, no group + } + let parent_link_path = parts[..parts.len() - 1].join("/"); + ensure_groups_for( + &parent_link_path, + &mut groups, + &mut group_index_map, + &mut group_headers, + ); + let gidx = group_index_map[&format!("/{}", parent_link_path)]; + groups[gidx].child_datasets.push(di); + } + + // Rebuild the hard-link registry from the alias entries set aside + // above, so the H5Ldelete semantics survive a reopen. An alias + // whose target header did not decode is dropped with its target + // (the primary path was skipped the same way). + let mut hard_links: Vec = Vec::new(); + for (path, addr) in alias_entries { + let target = if let Some(di) = existing_datasets + .iter() + .position(|d| d.obj_header_addr == addr) + { + HardLinkTarget::Dataset(di) + } else if let Some(gi) = groups + .iter() + .position(|g| g.obj_header_written_addr == Some(addr)) + { + HardLinkTarget::Group(gi) } else { - format!("/{}", parts[..parts.len() - 1].join("/")) + continue; }; - if let Some(&gidx) = group_index_map.get(&parent_path) { - groups[gidx].child_datasets.push(di); - } + let (parent, link_name) = match path.rsplit_once('/') { + None => (None, path), + Some((dir, leaf)) => { + ensure_groups_for(dir, &mut groups, &mut group_index_map, &mut group_headers); + ( + group_index_map.get(&format!("/{dir}")).copied(), + leaf.to_string(), + ) + } + }; + hard_links.push(HardLink { + parent, + name: link_name, + target, + }); } let allocator = FileAllocator::new(file_size); @@ -1213,14 +1771,16 @@ impl Hdf5Writer { ctx, datasets: Slot::new(datasets), groups: Slot::new(groups), - hard_links: Slot::new(Vec::new()), + hard_links: Slot::new(hard_links), root_attributes: Slot::new(root_attributes), create_lock: Slot::new(()), libver_latest: false, closed: false, swmr_active: false, + cwfs: Slot::new(Vec::new()), root_group_addr: None, root_group_encoded_size: 0, + superseded_root_header: Some((root_addr, root_header_size as u64)), }) } @@ -1299,12 +1859,28 @@ impl Hdf5Writer { .collect() } - /// Find a dataset index by name. + /// Find a dataset index by name. Like `H5Dopen`, the name may be any + /// link path to the dataset: a user hard link's path — or a path + /// whose group components pass through such links — resolves to its + /// target. pub fn dataset_index(&self, name: &str) -> Option { - self.dataset_refs().iter().position(|d| { - let g = d.lock(); - g.name == name && !g.deleted - }) + let name = self.canonical_dataset_path(name); + self.dataset_refs() + .iter() + .position(|d| { + let g = d.lock(); + g.name == name && !g.deleted + }) + .or_else(|| { + self.hard_links_vec().iter().find_map(|l| match l.target { + HardLinkTarget::Dataset(i) + if self.hard_link_emitted(l) && self.hard_link_full_path(l) == name => + { + Some(i) + } + _ => None, + }) + }) } /// Reconstruct the fields a writer-mode `H5Dataset` handle needs for the @@ -1353,65 +1929,575 @@ impl Hdf5Writer { Ok(()) } - /// Soft-delete a dataset by name. The dataset is excluded from the file - /// on close. File space is not reclaimed. + /// Delete a dataset name, with libhdf5's `H5Ldelete` semantics: a name + /// is only a link. If `name` is a user hard link's path, just that + /// link is removed and the object is untouched. If it is the tree name + /// and a user hard link still names the object, the object survives + /// under it — the link becomes the primary name and nothing is freed. + /// Only deleting the *last* name soft-deletes the object and frees the + /// file space it owned: its chunk blocks and chunk-index structures + /// (or contiguous data block), the global-heap objects of its + /// variable-length data and attributes, and — on a reopened file — the + /// on-disk object header block. The freed space is reused by later + /// allocations in this session; the file does not shrink. + /// + /// Refused while SWMR streaming is active: a live reader may hold any + /// of those addresses (libhdf5 forbids link deletion during SWMR + /// writes too). pub fn delete_dataset(&self, name: &str) -> IoResult<()> { + if self.swmr_active { + return Err(swmr_delete_error(name)); + } + // The gate keeps the link list and child lists still while this + // delete reads and rewrites them (create_lock → op → slot order, + // the same as every creator). + let _create = self.create_lock.lock(); + // `H5Ldelete` resolves the path through links only *up to* the + // leaf — the leaf is what gets deleted, so a leaf naming a user + // link must stay literal and be unlinked, not its target. + let name = match name.rsplit_once('/') { + None => name.to_string(), + Some((dir, leaf)) => format!( + "{}/{leaf}", + self.canonical_group_path(&format!("/{dir}")) + .trim_start_matches('/') + ), + }; let refs = self.dataset_refs(); - let idx = refs - .iter() - .position(|d| { - let g = d.lock(); - g.name == name && !g.deleted - }) - .ok_or_else(|| crate::io::IoError::NotFound(name.to_string()))?; + let idx = match refs.iter().position(|d| { + let g = d.lock(); + g.name == name && !g.deleted + }) { + Some(i) => i, + None => { + // Not a tree name — the path may name a user hard link, + // and deleting a link path unlinks just that link (the + // creation collision checks keep the two namespaces + // disjoint, so the order of the lookups cannot matter). + let link = self.hard_links_vec().iter().position(|l| { + self.hard_link_emitted(l) + && matches!(l.target, HardLinkTarget::Dataset(_)) + && self.hard_link_full_path(l) == name + }); + let Some(pos) = link else { + return Err(crate::io::IoError::NotFound(name)); + }; + self.hard_links.lock().remove(pos); + return Ok(()); + } + }; + // A surviving hard link keeps the object: promote the first one to + // the primary name and delete nothing. + let promote = self.hard_links_vec().iter().position(|l| { + self.hard_link_emitted(l) && matches!(l.target, HardLinkTarget::Dataset(i) if i == idx) + }); + if let Some(pos) = promote { + self.promote_dataset_to_link(idx, pos); + return Ok(()); + } refs[idx].lock().deleted = true; // Remove from parent group's child_datasets for grp in self.group_refs() { grp.lock().child_datasets.retain(|&di| di != idx); } - Ok(()) - } - - /// Soft-delete a group and all its child datasets and sub-groups. - /// File space is not reclaimed. + self.purge_dead_hard_links(); + let ds = self.ds(idx); + let _op = ds.op.lock(); + self.release_dataset_storage(idx) + } + + /// Soft-delete a group and all its child datasets and sub-groups, + /// freeing every deleted object's file space the way + /// [`delete_dataset`](Self::delete_dataset) does — with the same + /// `H5Ldelete` semantics: a `name` that is a user hard link's path + /// unlinks just that link, and hard links from *outside* the subtree + /// keep their targets. A dataset or group such a link names survives, + /// re-homed under the link (a group brings its whole subtree with + /// it); a link naming the deleted group itself turns the call into a + /// pure rename and nothing is freed. Refused while SWMR streaming is + /// active, same rule as `delete_dataset`. pub fn delete_group(&self, name: &str) -> IoResult<()> { + if self.swmr_active { + return Err(swmr_delete_error(name)); + } + // Same gate as `delete_dataset`: the pre-scan below and the + // promotions must see a still link list and child lists. + let _create = self.create_lock.lock(); let name = if name.starts_with('/') { name.to_string() } else { format!("/{}", name) }; + // Leaf stays literal, directory resolves through links — the + // same `H5Ldelete` rule as `delete_dataset`. + let name = match name.rsplit_once('/') { + Some((dir, leaf)) if !dir.is_empty() => { + format!("{}/{leaf}", self.canonical_group_path(dir)) + } + _ => name, + }; let groups = self.group_refs(); - let gidx = groups - .iter() - .position(|g| { - let gg = g.lock(); - gg.name == name && !gg.deleted - }) - .ok_or_else(|| crate::io::IoError::NotFound(name.clone()))?; - self.delete_group_recursive(gidx); + let gidx = match groups.iter().position(|g| { + let gg = g.lock(); + gg.name == name && !gg.deleted + }) { + Some(i) => i, + None => { + // Same `H5Ldelete` rule as `delete_dataset`: a path naming + // a user hard link to a group unlinks just that link. + let trimmed = name.trim_start_matches('/'); + let link = self.hard_links_vec().iter().position(|l| { + self.hard_link_emitted(l) + && matches!(l.target, HardLinkTarget::Group(_)) + && self.hard_link_full_path(l) == trimmed + }); + let Some(pos) = link else { + return Err(crate::io::IoError::NotFound(name.clone())); + }; + self.hard_links.lock().remove(pos); + return Ok(()); + } + }; + + // A link is "outside" when its parent group does not die with the + // subtree; only outside links can keep their targets alive. + fn outside(parent: Option, doomed_gs: &[usize]) -> bool { + match parent { + None => true, + Some(pi) => !doomed_gs.contains(&pi), + } + } + // A group an outside link names survives, re-homed with its whole + // subtree under the link. Each promotion moves that subtree out of + // the doomed set — and can turn a link inside it into an outside + // one — so rescan from scratch until no promotable group is left. + // Promoting `gidx` itself makes the delete a pure rename: return. + let mut doomed_ds = Vec::new(); + let mut doomed_gs = Vec::new(); + loop { + doomed_ds.clear(); + doomed_gs.clear(); + self.collect_live_subtree(gidx, &mut doomed_ds, &mut doomed_gs); + let promote = self + .hard_links_vec() + .iter() + .enumerate() + .find_map(|(pos, l)| match l.target { + HardLinkTarget::Group(gi) + if self.hard_link_emitted(l) + && outside(l.parent, &doomed_gs) + && doomed_gs.contains(&gi) => + { + Some((pos, gi)) + } + _ => None, + }); + let Some((pos, gi)) = promote else { break }; + self.promote_group_to_link(gi, pos); + if gi == gidx { + return Ok(()); + } + } + // A dataset an outside link names survives its container: re-home + // it under the link now, so the marking pass below never sees it. + for di in doomed_ds { + let promote = self.hard_links_vec().iter().position(|l| { + self.hard_link_emitted(l) + && outside(l.parent, &doomed_gs) + && matches!(l.target, HardLinkTarget::Dataset(i) if i == di) + }); + if let Some(pos) = promote { + self.promote_dataset_to_link(di, pos); + } + } + + let mut ds_deleted = Vec::new(); + let mut gs_deleted = Vec::new(); + self.delete_group_recursive(gidx, &mut ds_deleted, &mut gs_deleted); // Remove from parent's child_groups let parent = groups[gidx].lock().parent; if let Some(pidx) = parent { groups[pidx].lock().child_groups.retain(|&gi| gi != gidx); } + self.purge_dead_hard_links(); + // Free storage only after the whole subtree is marked: the lists + // hold each object exactly once (the marking pass skips anything + // already deleted), so nothing is freed twice. + for di in ds_deleted { + let ds = self.ds(di); + let _op = ds.op.lock(); + self.release_dataset_storage(di)?; + } + for gi in gs_deleted { + self.release_group_storage(gi)?; + } Ok(()) } - fn delete_group_recursive(&self, gidx: usize) { + /// Collect the live (not soft-deleted) members of `gidx`'s subtree, + /// each exactly once, without changing anything — the read-only twin + /// of [`delete_group_recursive`](Self::delete_group_recursive), for + /// the pre-scan that must run before any marking. + fn collect_live_subtree(&self, gidx: usize, ds_out: &mut Vec, gs_out: &mut Vec) { + if gs_out.contains(&gidx) { + return; + } + let (child_ds, child_gs) = { + let grp = self.grp(gidx); + let g = grp.lock(); + if g.deleted { + return; + } + (g.child_datasets.clone(), g.child_groups.clone()) + }; + gs_out.push(gidx); + for di in child_ds { + if !self.ds(di).lock().deleted && !ds_out.contains(&di) { + ds_out.push(di); + } + } + for gi in child_gs { + self.collect_live_subtree(gi, ds_out, gs_out); + } + } + + /// Re-home dataset `idx` under the hard link at `pos` in the link + /// list — the surviving half of `H5Ldelete`: the link leaves the user + /// list and becomes the dataset's primary (tree) name, in the link's + /// parent group. Storage is untouched; any further links to the + /// dataset stay in the list and keep resolving. + fn promote_dataset_to_link(&self, idx: usize, pos: usize) { + let link = self.hard_links.lock().remove(pos); + let new_name = self.hard_link_full_path(&link); + for grp in self.group_refs() { + grp.lock().child_datasets.retain(|&di| di != idx); + } + if let Some(pi) = link.parent { + self.grp(pi).lock().child_datasets.push(idx); + } + self.ds(idx).lock().name = new_name; + } + + /// The group counterpart of + /// [`promote_dataset_to_link`](Self::promote_dataset_to_link): re-home + /// group `gidx` under the hard link at `pos`, bringing its whole + /// subtree with it. Names are stored as full paths, so every live + /// descendant is renamed by prefix. + fn promote_group_to_link(&self, gidx: usize, pos: usize) { + let link = self.hard_links.lock().remove(pos); + let new_name = format!("/{}", self.hard_link_full_path(&link)); + let old_name = self.grp(gidx).lock().name.clone(); + for grp in self.group_refs() { + grp.lock().child_groups.retain(|&g| g != gidx); + } + { + let grp = self.grp(gidx); + let mut g = grp.lock(); + g.parent = link.parent; + g.name = new_name.clone(); + } + if let Some(pi) = link.parent { + self.grp(pi).lock().child_groups.push(gidx); + } + + let mut ds_in = Vec::new(); + let mut gs_in = Vec::new(); + self.collect_live_subtree(gidx, &mut ds_in, &mut gs_in); + // Group names carry a leading '/' ("/a/b"), dataset names none + // ("a/b/ds") — two prefix forms of the same rename. + let old_grp_prefix = format!("{old_name}/"); + let new_grp_prefix = format!("{new_name}/"); + let old_ds_prefix = old_grp_prefix.trim_start_matches('/').to_string(); + let new_ds_prefix = new_grp_prefix.trim_start_matches('/').to_string(); + for gi in gs_in { + if gi == gidx { + continue; + } + let grp = self.grp(gi); + let mut g = grp.lock(); + let renamed = g + .name + .strip_prefix(&old_grp_prefix) + .map(|rest| format!("{new_grp_prefix}{rest}")); + if let Some(n) = renamed { + g.name = n; + } + } + for di in ds_in { + let ds = self.ds(di); + let mut d = ds.lock(); + let renamed = d + .name + .strip_prefix(&old_ds_prefix) + .map(|rest| format!("{new_ds_prefix}{rest}")); + if let Some(n) = renamed { + d.name = n; + } + } + } + + /// Drop hard-link entries that can no longer be emitted — their parent + /// group or target object was just deleted — so the list mirrors what + /// the file will hold instead of carrying suppressed zombies. + fn purge_dead_hard_links(&self) { + let dead: Vec = self + .hard_links_vec() + .iter() + .enumerate() + .filter(|(_, l)| !self.hard_link_emitted(l)) + .map(|(p, _)| p) + .collect(); + let mut links = self.hard_links.lock(); + for p in dead.into_iter().rev() { + links.remove(p); + } + } + + /// Mark `gidx` and its subtree deleted, appending each newly-deleted + /// object's index to `ds_out` / `gs_out` exactly once — the caller + /// frees their storage, and an object reachable twice (or a subtree + /// already deleted) must not be freed twice. + fn delete_group_recursive( + &self, + gidx: usize, + ds_out: &mut Vec, + gs_out: &mut Vec, + ) { // Mark deleted and snapshot the child lists, releasing the group lock // before locking any dataset/child-group slot (spine → slot order). let (child_ds, child_gs) = { let grp = self.grp(gidx); let mut g = grp.lock(); + if g.deleted { + return; + } g.deleted = true; (g.child_datasets.clone(), g.child_groups.clone()) }; + gs_out.push(gidx); for di in child_ds { - self.ds(di).lock().deleted = true; + let ds = self.ds(di); + let mut d = ds.lock(); + if !d.deleted { + d.deleted = true; + ds_out.push(di); + } } for gi in child_gs { - self.delete_group_recursive(gi); + self.delete_group_recursive(gi, ds_out, gs_out); + } + } + + /// Free everything a soft-deleted dataset owned. The single owner of + /// delete-time reclamation, called only from the two delete paths with + /// the dataset already marked deleted and its op lock held. + /// + /// A deleted dataset contributes nothing to finalize (the header, + /// index-flush and append-flush loops all skip it), so nothing in the + /// finalized file can reference the blocks freed here. Never runs under + /// SWMR — the delete entry points refuse first. + fn release_dataset_storage(&self, index: usize) -> IoResult<()> { + use crate::format::messages::datatype::DatatypeMessage; + let (indexed, ndims, contiguous, is_vlen, attrs, header_block) = { + let ds = self.ds(index); + let mut m = ds.lock(); + // Buffered rows were never written to a chunk; they die with + // the dataset instead of being flushed at close. + m.append = None; + let indexed = m.chunked.is_some() || m.fixed_array.is_some() || m.btree_v2.is_some(); + let contiguous = (!indexed && m.data_addr != UNDEF_ADDR && m.data_size > 0) + .then_some((m.data_addr, m.data_size)); + m.data_addr = UNDEF_ADDR; + m.data_size = 0; + let is_vlen = matches!( + m.datatype, + DatatypeMessage::VarLenString { .. } | DatatypeMessage::VarLenSequence { .. } + ); + let attrs = std::mem::take(&mut m.attributes); + let header_block = m + .obj_header_written_addr + .take() + .filter(|_| m.obj_header_encoded_size > 0) + .map(|a| (a, m.obj_header_encoded_size as u64)); + m.obj_header_encoded_size = 0; + ( + indexed, + m.dataspace.dims.len(), + contiguous, + is_vlen, + attrs, + header_block, + ) + }; + if indexed { + // Prune to a zero extent: every stored chunk is entirely beyond + // it, so the walk frees each chunk block and collects the vlen + // references its bytes held (released inside). + self.prune_chunks_beyond(index, &vec![0; ndims])?; + self.free_chunk_index(index)?; + } else if let Some((addr, size)) = contiguous { + if is_vlen { + let data = self.handle.read_at(addr, size as usize)?; + self.release_vlen_references(&data)?; + } + self.allocator.free(addr, size); + } + for attr in &attrs { + self.release_attr_vlen(attr)?; + } + if let Some((addr, size)) = header_block { + self.allocator.free(addr, size); + } + Ok(()) + } + + /// Free a deleted group's file space: its attributes' global-heap + /// objects and, on a reopened file, the on-disk header block. The + /// group counterpart of + /// [`release_dataset_storage`](Self::release_dataset_storage). + fn release_group_storage(&self, gidx: usize) -> IoResult<()> { + let (attrs, header_block) = { + let grp = self.grp(gidx); + let mut g = grp.lock(); + let attrs = std::mem::take(&mut g.attributes); + let header_block = g + .obj_header_written_addr + .take() + .filter(|_| g.obj_header_encoded_size > 0) + .map(|a| (a, g.obj_header_encoded_size as u64)); + g.obj_header_encoded_size = 0; + (attrs, header_block) + }; + for attr in &attrs { + self.release_attr_vlen(attr)?; + } + if let Some((addr, size)) = header_block { + self.allocator.free(addr, size); + } + Ok(()) + } + + /// Free a deleted dataset's chunk-index structures, after the chunks + /// themselves were freed by a zero-extent prune. Takes the index info + /// out of the slot, so the dataset no longer claims chunked storage. + /// + /// Every block's size is recovered the way its allocation computed it: + /// re-encoding the in-memory copy (EA header and index block, FA + /// header and data block, BT2 header) or sizing a same-shape dummy + /// from the array geometry (EA data blocks, whose element counts come + /// from [`EaGeometry`]; BT2 nodes are all `node_size`). + fn free_chunk_index(&self, index: usize) -> IoResult<()> { + let ds = self.ds(index); + let mut m = ds.lock(); + let is_filtered = m.filter_pipeline.is_some(); + if let Some(c) = m.chunked.take() { + let p = &c.earray_params; + let bits = p.max_nelmts_bits; + let csl = c.chunk_size_len; + let geo = EaGeometry::new( + p.idx_blk_elmts, + p.data_blk_min_elmts, + p.sup_blk_min_data_ptrs, + bits, + p.max_dblk_page_nelmts_bits, + )?; + let dblk_size = |nelmts: u64| -> u64 { + if is_filtered { + FilteredDataBlock::new(c.ea_header_addr, 0, nelmts as usize) + .encode(&self.ctx, bits, csl) + .len() as u64 + } else { + ExtensibleArrayDataBlock::new(c.ea_header_addr, 0, nelmts as usize) + .encoded_size(&self.ctx, bits) as u64 + } + }; + let (dblk_addrs, sblk_addrs, iblk_size) = if is_filtered { + let f = c.filt_iblk.as_ref().unwrap(); + ( + f.dblk_addrs.clone(), + f.sblk_addrs.clone(), + f.encode(&self.ctx, csl).len() as u64, + ) + } else { + ( + c.ea_iblk.dblk_addrs.clone(), + c.ea_iblk.sblk_addrs.clone(), + c.ea_iblk.encoded_size(&self.ctx) as u64, + ) + }; + // Data blocks addressed from the index block belong to the + // first `iblock_nsblks` super blocks; each of those defines the + // element count (and so the disk size) of its data blocks. + let mut g = 0usize; + 'direct: for s in geo.sblk.iter().take(geo.iblock_nsblks) { + for _ in 0..s.ndblks { + let Some(&a) = dblk_addrs.get(g) else { + break 'direct; + }; + g += 1; + if a == UNDEF_ADDR { + continue; + } + if s.dblk_nelmts > geo.dblk_page_nelmts { + return Err(crate::io::IoError::InvalidState( + "cannot free a paged extensible-array data block, \ + which is not yet supported" + .into(), + )); + } + self.allocator.free(a, dblk_size(s.dblk_nelmts)); + } + } + for (off, &sa) in sblk_addrs.iter().enumerate() { + if sa == UNDEF_ADDR { + continue; + } + let s = geo.sblk[geo.iblock_nsblks + off]; + if s.dblk_nelmts > geo.dblk_page_nelmts { + return Err(crate::io::IoError::InvalidState( + "cannot free a paged extensible-array data block, \ + which is not yet supported" + .into(), + )); + } + let buf = self.handle.read_at_most(sa, 65536)?; + let sb = + ExtensibleArraySuperBlock::decode(&buf, &self.ctx, bits, s.ndblks as usize, 0)?; + for &da in &sb.dblk_addrs { + if da != UNDEF_ADDR { + self.allocator.free(da, dblk_size(s.dblk_nelmts)); + } + } + self.allocator + .free(sa, sb.encode(&self.ctx, bits).len() as u64); + } + self.allocator.free(c.ea_iblk_addr, iblk_size); + self.allocator + .free(c.ea_header_addr, c.ea_header.encoded_size(&self.ctx) as u64); + return Ok(()); } + if let Some(fa) = m.fixed_array.take() { + self.allocator.free( + fa.fa_dblk_addr, + fixed_array_dblk_disk_size(&self.ctx, &fa.fa_header), + ); + self.allocator.free( + fa.fa_header_addr, + fa.fa_header.encode(&self.ctx).len() as u64, + ); + return Ok(()); + } + if let Some(bt2) = m.btree_v2.take() { + let tree = bt2.index.build_tree(&self.ctx); + for &a in &bt2.node_addrs { + self.allocator.free(a, tree.node_size as u64); + } + self.allocator.free( + bt2.bt2_header_addr, + tree.header(UNDEF_ADDR).encode(&self.ctx).len() as u64, + ); + } + Ok(()) } /// Return the chunk dimensions for a dataset, if chunked. @@ -1456,6 +2542,10 @@ impl Hdf5Writer { // Hold the create gate across the uniqueness check and the registry // push so the two are atomic (see `create_lock`). let _create = self.create_lock.lock(); + // A parent path through hard links creates in the link's target, + // as HDF5 traversal does. + let parent_path = self.canonical_group_path(parent_path); + let parent_path = parent_path.as_str(); let full_name = if parent_path == "/" { format!("/{}", name) } else { @@ -1494,7 +2584,10 @@ impl Hdf5Writer { } else { let idx = groups .iter() - .position(|g| g.lock().name == parent_path) + .position(|g| { + let gg = g.lock(); + gg.name == parent_path && !gg.deleted + }) .ok_or_else(|| { crate::io::IoError::NotFound(format!( "parent group '{}' not found", @@ -1510,6 +2603,8 @@ impl Hdf5Writer { child_datasets: Vec::new(), child_groups: Vec::new(), obj_header_addr: 0, + obj_header_written_addr: None, + obj_header_encoded_size: 0, deleted: false, attributes: Vec::new(), }); @@ -1527,10 +2622,15 @@ impl Hdf5Writer { /// `group_path` is the full path of the group (e.g., "/detector"). /// `ds_index` is the dataset index returned by `create_dataset`. pub fn assign_dataset_to_group(&self, group_path: &str, ds_index: usize) -> IoResult<()> { + let group_path = self.canonical_group_path(group_path); + let group_path = group_path.as_str(); let groups = self.group_refs(); let group_idx = groups .iter() - .position(|g| g.lock().name == group_path) + .position(|g| { + let gg = g.lock(); + gg.name == group_path && !gg.deleted + }) .ok_or_else(|| { crate::io::IoError::NotFound(format!("group '{}' not found", group_path)) })?; @@ -1564,6 +2664,9 @@ impl Hdf5Writer { // Hold the create gate across the collision check and the hard-link // push so the two are atomic (see `create_lock`). let _create = self.create_lock.lock(); + // Both paths resolve through hard links, as HDF5 traversal does. + let parent_group_path = self.canonical_group_path(parent_group_path); + let parent_group_path = parent_group_path.as_str(); let datasets = self.dataset_refs(); let groups = self.group_refs(); @@ -1590,7 +2693,8 @@ impl Hdf5Writer { // Resolve the target. Dataset names are stored without a leading // '/', group names with one — compare on the trimmed form. A // trailing '/' is tolerated too. - let target_rel = target_path.trim_matches('/'); + let target_rel = self.canonical_dataset_path(target_path.trim_matches('/')); + let target_rel = target_rel.as_str(); if target_rel.is_empty() { return Err(crate::io::IoError::InvalidState( "cannot hard-link the root group".into(), @@ -1606,6 +2710,14 @@ impl Hdf5Writer { !gg.deleted && gg.name.trim_start_matches('/') == target_rel }) { HardLinkTarget::Group(idx) + } else if let Some(t) = self.hard_links_vec().iter().find_map(|l| { + (self.hard_link_emitted(l) && self.hard_link_full_path(l) == target_rel) + .then_some(l.target) + }) { + // The target path may itself be a hard link: links have no + // chain (all point straight at the object header, as in + // libhdf5), so the new link copies the existing one's target. + t } else { return Err(crate::io::IoError::NotFound(format!( "hard link target '{target_path}' not found" @@ -1669,6 +2781,49 @@ impl Hdf5Writer { } } + /// Rewrite a group path that passes through hard links into the tree + /// path of the group it reaches — HDF5 traversal, where any link in a + /// path component resolves to its target. Group-name form (leading + /// `/`). Repeats because a substituted target's subtree can hold + /// further links; bounded like libhdf5's link-traversal limit, so a + /// link cycle cannot loop forever. A path with no link components + /// (including one naming nothing at all) comes back unchanged. + pub(crate) fn canonical_group_path(&self, path: &str) -> String { + let mut path = path.to_string(); + for _ in 0..64 { + // The longest emitted group-link path that is the whole of + // `path` or a '/'-boundary prefix of it. + let mut best: Option<(usize, usize)> = None; // (prefix len, target) + for l in self.hard_links_vec() { + let HardLinkTarget::Group(gi) = l.target else { + continue; + }; + if !self.hard_link_emitted(&l) { + continue; + } + let lp = format!("/{}", self.hard_link_full_path(&l)); + let covers = path == lp || path.starts_with(&format!("{lp}/")); + if covers && best.is_none_or(|(len, _)| lp.len() > len) { + best = Some((lp.len(), gi)); + } + } + let Some((len, gi)) = best else { break }; + let target_name = self.grp(gi).lock().name.clone(); + path = format!("{}{}", target_name, &path[len..]); + } + path + } + + /// [`canonical_group_path`](Self::canonical_group_path) in the + /// dataset-name form (no leading `/`): the leaf is a dataset, so only + /// group links can appear as components and the whole path can go + /// through the group rewrite unchanged. + fn canonical_dataset_path(&self, name: &str) -> String { + self.canonical_group_path(&format!("/{name}")) + .trim_start_matches('/') + .to_string() + } + /// Total number of hard links resolving to an object: its own tree link /// plus every emitted user-created hard link pointing at it. fn object_link_count(&self, target: HardLinkTarget) -> u32 { @@ -1717,6 +2872,7 @@ impl Hdf5Writer { dims: &[u64], ) -> IoResult { let create = self.begin_create(name)?; + let name = create.name.as_str(); let total_elements: u64 = if dims.is_empty() { 1 } else { @@ -1756,6 +2912,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: None, deleted: false, + extent_dirty: false, fill_value: None, layout_version: 4, }, @@ -1778,6 +2935,7 @@ impl Hdf5Writer { chunk_dims: &[u64], ) -> IoResult { let create = self.begin_create(name)?; + let name = create.name.as_str(); validate_chunk_geometry(dims, max_dims, chunk_dims)?; ensure_unlimited_is_leading(max_dims)?; let chunk_bytes = chunk_dims.iter().product::() * datatype.element_size() as u64; @@ -1843,6 +3001,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: None, deleted: false, + extent_dirty: false, fill_value: None, layout_version, fixed_array: None, @@ -2500,51 +3659,372 @@ impl Hdf5Writer { coords[d] += 1; break; } - coords[d] = first[d]; + coords[d] = first[d]; + } + } + } + + /// Add an attribute to the root group (file-level attribute), replacing + /// a same-name attribute. See [`set_attribute`](Self::set_attribute). + pub fn add_root_attribute(&self, attr: AttributeMessage) -> IoResult<()> { + self.set_attribute(AttrTarget::Root, attr) + } + + /// Insert `attr` into the attribute list `target` names, replacing a + /// same-name attribute. + /// + /// The single owner of attribute-list mutation: an `AttributeMessage` + /// that leaves a list here has its vlen global-heap objects released, so + /// no replacement — vlen over vlen, numeric over vlen — can strand heap + /// space (the attribute counterpart of issue #10's dataset fix). + /// + /// Under SWMR every attribute mutation is refused, matching libhdf5's + /// rule for SWMR writes. Object headers are frozen once streaming + /// starts — a change was committed at close only when the header + /// happened to be rebuilt (group attrs always, dataset attrs only if + /// the dataset also got chunk writes) and silently dropped otherwise — + /// and a replacement's superseded vlen value could never be reclaimed, + /// since a streaming reader may hold its heap references. + pub fn set_attribute(&self, target: AttrTarget<'_>, attr: AttributeMessage) -> IoResult<()> { + if self.swmr_active { + return Err(swmr_attr_error(&attr.name)); + } + let old = self.with_attr_list(target, |attrs| { + if let Some(pos) = attrs.iter().position(|a| a.name == attr.name) { + Some(std::mem::replace(&mut attrs[pos], attr)) + } else { + attrs.push(attr); + None + } + })?; + match old { + Some(old) => self.release_attr_vlen(&old), + None => Ok(()), + } + } + + /// Set a variable-length string attribute on `target`, replacing any + /// same-name attribute. + /// + /// Owns the whole replacement sequence: the superseded attribute is + /// removed and its heap objects released *before* the new value's + /// collection is allocated — the free-before-alloc order (issue #10) + /// that lets a reopen-replace loop land in the block it just freed + /// instead of growing the file every session. The cost, as on the + /// dataset path: a failure between the eviction and the insert below + /// loses the attribute rather than leaking its heap space. + pub fn set_vlen_string_attribute( + &self, + target: AttrTarget<'_>, + name: &str, + value: &str, + ) -> IoResult<()> { + self.evict_attr(target, name)?; + let attr = self.vlen_string_attribute(name, value)?; + self.set_attribute(target, attr) + } + + /// The array counterpart of + /// [`set_vlen_string_attribute`](Self::set_vlen_string_attribute). + pub fn set_vlen_string_array_attribute( + &self, + target: AttrTarget<'_>, + name: &str, + values: &[&str], + dims: &[u64], + ) -> IoResult<()> { + self.evict_attr(target, name)?; + let attr = self.vlen_string_array_attribute(name, values, dims)?; + self.set_attribute(target, attr) + } + + /// Take the attribute `name` off `target`'s list, releasing its heap + /// objects. No-op when absent. Refused under SWMR — see + /// [`set_attribute`](Self::set_attribute). + fn evict_attr(&self, target: AttrTarget<'_>, name: &str) -> IoResult<()> { + if self.swmr_active { + return Err(swmr_attr_error(name)); + } + let old = self.with_attr_list(target, |attrs| { + attrs + .iter() + .position(|a| a.name == name) + .map(|pos| attrs.remove(pos)) + })?; + match old { + Some(old) => self.release_attr_vlen(&old), + None => Ok(()), + } + } + + /// Release the global-heap objects a superseded attribute owned. + /// Recognizes top-level vlen datatypes only: a *compound* attribute + /// with vlen members — which this crate cannot write, only a foreign + /// file can carry — keeps its members' heap objects when replaced or + /// deleted, the storage cost the foreign writer accepted. Every other + /// class stores its value inline in the message. Per-object removal + /// keeps collections shared with other refs (libhdf5-written files) + /// intact. + fn release_attr_vlen(&self, old: &AttributeMessage) -> IoResult<()> { + use crate::format::messages::datatype::DatatypeMessage; + if matches!( + old.datatype, + DatatypeMessage::VarLenString { .. } | DatatypeMessage::VarLenSequence { .. } + ) { + self.release_vlen_references(&old.data)?; + } + Ok(()) + } + + /// Run `f` on the attribute list `target` names — the accessor every + /// attribute mutation shares. + fn with_attr_list( + &self, + target: AttrTarget<'_>, + f: impl FnOnce(&mut Vec) -> R, + ) -> IoResult { + match target { + AttrTarget::Root => Ok(f(&mut self.root_attributes.lock())), + AttrTarget::Group(path) => { + let path = self.canonical_group_path(path); + for grp in self.group_refs() { + let mut g = grp.lock(); + if g.name == path && !g.deleted { + return Ok(f(&mut g.attributes)); + } + } + Err(crate::io::IoError::NotFound(format!( + "group '{path}' not found" + ))) + } + AttrTarget::Dataset(index) => { + let count = self.dataset_count(); + if index >= count { + return Err(crate::io::IoError::InvalidState(format!( + "dataset index {index} out of range (have {count})" + ))); + } + Ok(f(&mut self.ds(index).lock().attributes)) + } + } + } + + /// Store each of `items` as a global heap object and return its + /// placement `(collection address, object index)`, in input order — + /// the writer side of libhdf5's `H5HG_insert`. + /// + /// Placement follows libhdf5: a collection from the CWFS list takes an + /// item when its free space holds the object *and* a residual + /// free-space marker header (`encode_at_size` always emits the + /// marker); what no listed collection can take goes into a fresh + /// collection, spilling into another at the 65535-object index cap. + /// One batch may therefore span several collections — invisible to + /// readers, which resolve each reference's own collection address. An + /// empty batch allocates nothing: an empty collection still encodes + /// to the 4096-byte `H5HG_MINALLOC` minimum, a block nothing would + /// reference. libhdf5 additionally tries to extend a nearly-full + /// collection's block in place (`H5MF_try_extend`); this writer does + /// not — an oversized item always starts a fresh collection. + /// + /// The `cwfs` lock is held across every read-modify-rewrite of a + /// listed collection block: it serializes concurrent inserts (two + /// datasets' writers can pack the same block) and inserts against + /// [`release_vlen_references`](Self::release_vlen_references), which + /// rewrites the same blocks when objects are freed. + /// + /// Under SWMR the CWFS list is neither consulted nor updated and every + /// batch gets fresh collections: packing rewrites a block a streaming + /// reader may be mid-walk on — the same reason `place_chunk` keeps a + /// relocated chunk's old block. + fn insert_vlen_objects(&self, items: &[&[u8]]) -> IoResult> { + use crate::format::global_heap::{GlobalHeapCollection, GlobalHeapObject}; + + if items.is_empty() { + return Ok(Vec::new()); + } + let objhdr = GlobalHeapCollection::object_disk_size(&self.ctx, 0); + let mut placements = Vec::with_capacity(items.len()); + let mut i = 0; + + // Pack into listed collections while one can take the next item. + if !self.swmr_active { + let mut cwfs = self.cwfs.lock(); + while i < items.len() { + let need = GlobalHeapCollection::object_disk_size(&self.ctx, items[i].len()); + let Some(pos) = cwfs.iter().position(|e| e.free >= need + objhdr) else { + // Second pass of libhdf5's H5F_cwfs_find_free_heap: no + // listed collection has room, so try to grow one in + // place before falling back to a fresh collection. + if self.extend_listed_collection(&mut cwfs, need + objhdr)? { + continue; + } + break; + }; + let (addr, size) = (cwfs[pos].addr, cwfs[pos].size); + let image = self.handle.read_at(addr, size)?; + let (mut gcol, _) = GlobalHeapCollection::decode(&image[..size], &self.ctx)?; + // The disk is the truth for free space; the entry is a hint. + let Some(mut free) = gcol.free_space_at(&self.ctx, size) else { + cwfs.remove(pos); + continue; + }; + let mut next_idx = gcol.max_index(); + let mut took = false; + while i < items.len() && next_idx < u16::MAX { + let need = GlobalHeapCollection::object_disk_size(&self.ctx, items[i].len()); + if free < need + objhdr { + break; + } + next_idx += 1; + gcol.objects.push(GlobalHeapObject { + index: next_idx, + ref_count: 0, + data: items[i].to_vec(), + }); + placements.push((addr, next_idx)); + free -= need; + took = true; + i += 1; + } + if took { + let rewritten = gcol.encode_at_size(&self.ctx, size)?; + self.handle.write_at(addr, &rewritten)?; + // Correct the entry to the measured free space and move + // it to the front — libhdf5 keeps `cwfs` in + // most-recently-used order. + let mut e = cwfs.remove(pos); + e.free = free; + cwfs.insert(0, e); + } else if next_idx == u16::MAX { + // At the index cap nothing can be inserted no matter the + // free space; drop the entry or the scan re-picks it + // forever. (A removal can lower the top index again, and + // the release side re-lists the collection then.) + cwfs.remove(pos); + } else { + // The hint overstated the block's free space — shrink it + // to the measured value so the scan moves on. + cwfs[pos].free = free; + } + } + } + + // What remains goes into fresh collections. + while i < items.len() { + let mut gcol = GlobalHeapCollection::new(); + // Objects are pushed with a running index: `add_object` rescans + // for the max index per call, O(n²) across a spill-sized batch. + let mut next_idx: u16 = 0; + while i < items.len() && next_idx < u16::MAX { + next_idx += 1; + gcol.objects.push(GlobalHeapObject { + index: next_idx, + ref_count: 0, + data: items[i].to_vec(), + }); + i += 1; + } + let encoded = gcol.encode(&self.ctx); + let addr = self.allocator.allocate(encoded.len() as u64); + self.handle.write_at(addr, &encoded)?; + for idx in 1..=next_idx { + placements.push((addr, idx)); + } + // List the block's leftover free space for later inserts — the + // minimum-size padding of a small batch is most of 4096 bytes. + // Below two object headers not even an empty object fits. + if !self.swmr_active { + if let Some(free) = gcol.free_space_at(&self.ctx, encoded.len()) { + if free >= 2 * objhdr { + cwfs_note(&mut self.cwfs.lock(), addr, encoded.len(), free); + } + } } } + Ok(placements) } - /// Add an attribute to the root group (file-level attribute). - pub fn add_root_attribute(&self, attr: crate::format::messages::attribute::AttributeMessage) { - // Replace existing attribute with the same name, or append new one. - let mut attrs = self.root_attributes.lock(); - if let Some(pos) = attrs.iter().position(|a| a.name == attr.name) { - attrs[pos] = attr; - } else { - attrs.push(attr); + /// Try to extend one listed collection in place so it can take an + /// object needing `want` bytes of free space — the second pass of + /// libhdf5's `H5F_cwfs_find_free_heap`: grow the file allocation + /// ([`FileAllocator::try_extend`], mirroring `H5MF_try_extend`) and then + /// the collection itself (`H5HG_extend`: a larger declared size and a + /// free-space marker covering the new tail — here by re-encoding at the + /// grown size, which writes exactly those two things). + /// + /// Extension size is `max(collection_size, shortfall)` — at least a + /// doubling — capped so the result stays within [`GCOL_MAX_SIZE`], both + /// as upstream computes them. On success the grown entry moves to the + /// front of the list and the caller's scan re-picks it; the free-space + /// measurement is taken from the block on disk, not the list's hint, so + /// the rewrite and the entry agree. + /// + /// Caller holds the `cwfs` lock (it passes the guarded list), which is + /// what serializes this read-modify-rewrite against concurrent inserts + /// and releases. + fn extend_listed_collection(&self, cwfs: &mut Vec, want: usize) -> IoResult { + use crate::format::global_heap::{GlobalHeapCollection, GCOL_MAX_SIZE}; + + let mut pos = 0; + while pos < cwfs.len() { + let (addr, size) = (cwfs[pos].addr, cwfs[pos].size); + let image = self.handle.read_at(addr, size)?; + let (gcol, _) = GlobalHeapCollection::decode(&image[..size], &self.ctx)?; + // The disk is the truth for free space; the entry is a hint. + let Some(free) = gcol.free_space_at(&self.ctx, size) else { + cwfs.remove(pos); + continue; + }; + // A hint can understate the block (upstream's FREE_SIZE is its + // in-memory truth and cannot): if the block already has room, + // correct the hint instead of doubling the collection. + if free >= want { + cwfs[pos].free = free; + return Ok(true); + } + let new_need = size.max(want.saturating_sub(free)); + if size + new_need > GCOL_MAX_SIZE + || !self + .allocator + .try_extend(addr, size as u64, new_need as u64) + { + pos += 1; + continue; + } + let new_size = size + new_need; + let rewritten = gcol.encode_at_size(&self.ctx, new_size)?; + self.handle.write_at(addr, &rewritten)?; + let mut e = cwfs.remove(pos); + e.size = new_size; + e.free = free + new_need; + cwfs.insert(0, e); + return Ok(true); } + Ok(false) } /// Create a variable-length string dataset and write string data. /// - /// Stores strings in a global heap collection. The dataset raw data - /// consists of vlen references (collection_addr + object_index pairs). + /// Stores strings in the global heap. The dataset raw data consists of + /// vlen references (collection_addr + object_index pairs). pub fn create_vlen_string_dataset(&self, name: &str, strings: &[&str]) -> IoResult { - use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + use crate::format::global_heap::encode_vlen_reference; use crate::format::messages::datatype::DatatypeMessage; let create = self.begin_create(name)?; + let name = create.name.as_str(); let num_strings = strings.len() as u64; - // Build a global heap collection with all strings - let mut gcol = GlobalHeapCollection::new(); - let mut obj_indices = Vec::with_capacity(strings.len()); - for s in strings { - let idx = gcol.add_object(s.as_bytes().to_vec())?; - obj_indices.push(idx); - } - - // Encode and write the global heap collection - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; + // Store the strings as heap objects; a batch that fits an earlier + // collection's free space shares its block. + let items: Vec<&[u8]> = strings.iter().map(|s| s.as_bytes()).collect(); + let placements = self.insert_vlen_objects(&items)?; // Build raw data: vlen references let ref_size = crate::format::global_heap::vlen_reference_size(&self.ctx); let data_size = (num_strings as usize) * ref_size; let mut raw_data = Vec::with_capacity(data_size); - for (i, &obj_idx) in obj_indices.iter().enumerate() { + for (i, &(gcol_addr, obj_idx)) in placements.iter().enumerate() { let seq_len = crate::format::global_heap::vlen_seq_len(strings[i].len())?; raw_data.extend_from_slice(&encode_vlen_reference( seq_len, @@ -2577,6 +4057,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: None, deleted: false, + extent_dirty: false, fill_value: None, layout_version: 4, chunked: None, @@ -2598,30 +4079,22 @@ impl Hdf5Writer { /// of base (`u8`) elements, i.e. the byte length; no null terminator is /// appended. pub fn create_vlen_bytes_dataset(&self, name: &str, items: &[&[u8]]) -> IoResult { - use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + use crate::format::global_heap::encode_vlen_reference; use crate::format::messages::datatype::DatatypeMessage; let create = self.begin_create(name)?; + let name = create.name.as_str(); let num_items = items.len() as u64; - // Build a global heap collection with all byte arrays. - let mut gcol = GlobalHeapCollection::new(); - let mut obj_indices = Vec::with_capacity(items.len()); - for item in items { - let idx = gcol.add_object(item.to_vec())?; - obj_indices.push(idx); - } - - // Encode and write the global heap collection. - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; + // Store the byte arrays as heap objects, sharing collection blocks + // as `create_vlen_string_dataset` does. + let placements = self.insert_vlen_objects(items)?; // Build raw data: one vlen reference per item. let ref_size = crate::format::global_heap::vlen_reference_size(&self.ctx); let data_size = (num_items as usize) * ref_size; let mut raw_data = Vec::with_capacity(data_size); - for (i, &obj_idx) in obj_indices.iter().enumerate() { + for (i, &(gcol_addr, obj_idx)) in placements.iter().enumerate() { // base is u8, so element count == byte count. let seq_len = crate::format::global_heap::vlen_seq_len(items[i].len())?; raw_data.extend_from_slice(&encode_vlen_reference( @@ -2654,6 +4127,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: None, deleted: false, + extent_dirty: false, fill_value: None, layout_version: 4, chunked: None, @@ -2678,31 +4152,24 @@ impl Hdf5Writer { chunk_size: usize, pipeline: FilterPipeline, ) -> IoResult { - use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + use crate::format::global_heap::encode_vlen_reference; use crate::format::messages::datatype::DatatypeMessage; let create = self.begin_create(name)?; + let name = create.name.as_str(); let num_strings = strings.len() as u64; validate_chunk_geometry(&[num_strings], &[num_strings], &[chunk_size as u64])?; - // Build a global heap collection with all strings - let mut gcol = GlobalHeapCollection::new(); - let mut obj_indices = Vec::with_capacity(strings.len()); - for s in strings { - let idx = gcol.add_object(s.as_bytes().to_vec())?; - obj_indices.push(idx); - } - - // Encode and write the global heap collection - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; + // Store the strings as heap objects; the geometry validation above + // must precede this so a refused call allocates nothing. + let items: Vec<&[u8]> = strings.iter().map(|s| s.as_bytes()).collect(); + let placements = self.insert_vlen_objects(&items)?; // Build raw data: vlen references let ref_size = crate::format::global_heap::vlen_reference_size(&self.ctx); let data_size = (num_strings as usize) * ref_size; let mut raw_data = Vec::with_capacity(data_size); - for (i, &obj_idx) in obj_indices.iter().enumerate() { + for (i, &(gcol_addr, obj_idx)) in placements.iter().enumerate() { let seq_len = crate::format::global_heap::vlen_seq_len(strings[i].len())?; raw_data.extend_from_slice(&encode_vlen_reference( seq_len, @@ -2785,6 +4252,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: Some(pipeline), deleted: false, + extent_dirty: false, fill_value: None, layout_version, fixed_array: None, @@ -2861,7 +4329,7 @@ impl Hdf5Writer { /// Creates a new global heap collection for the strings, builds vlen /// references, and appends them as new chunks to the dataset. pub fn append_vlen_strings(&self, ds_index: usize, strings: &[&str]) -> IoResult<()> { - use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + use crate::format::global_heap::encode_vlen_reference; use crate::format::messages::datatype::DatatypeMessage; if strings.is_empty() { @@ -2890,21 +4358,25 @@ impl Hdf5Writer { }; ensure_vlen_charset(charset, strings)?; - // Build a new global heap collection for this batch - let mut gcol = GlobalHeapCollection::new(); - let mut obj_indices = Vec::with_capacity(strings.len()); - for s in strings { - let idx = gcol.add_object(s.as_bytes().to_vec())?; - obj_indices.push(idx); - } - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; + // Every deterministic rejection must precede the heap write below: + // a collection written for a batch the append then refuses (a + // contiguous dataset, or a reopened dataset whose chunk index was + // not reconstructed) is a 4096-byte orphan nothing references. + let chunk_dims = self + .dataset_chunk_dims(ds_index) + .ok_or_else(|| crate::io::IoError::InvalidState("not a chunked dataset".into()))? + .to_vec(); + let dims = self.dataset_dims(ds_index).to_vec(); + + // Store the batch's strings as heap objects; a batch that fits an + // earlier collection's free space shares its block. + let items: Vec<&[u8]> = strings.iter().map(|s| s.as_bytes()).collect(); + let placements = self.insert_vlen_objects(&items)?; // Build raw vlen reference bytes let ref_size = crate::format::global_heap::vlen_reference_size(&self.ctx); let mut raw = Vec::with_capacity(strings.len() * ref_size); - for (i, &obj_idx) in obj_indices.iter().enumerate() { + for (i, &(gcol_addr, obj_idx)) in placements.iter().enumerate() { let seq_len = crate::format::global_heap::vlen_seq_len(strings[i].len())?; raw.extend_from_slice(&encode_vlen_reference( seq_len, @@ -2914,13 +4386,6 @@ impl Hdf5Writer { )); } - // Use the same chunked-append logic as append - let chunk_dims = self - .dataset_chunk_dims(ds_index) - .ok_or_else(|| crate::io::IoError::InvalidState("not a chunked dataset".into()))? - .to_vec(); - let dims = self.dataset_dims(ds_index).to_vec(); - let n_new_frames = strings.len(); let current_dim0 = dims[0] as usize; let chunk_dim0 = chunk_dims[0] as usize; @@ -2983,11 +4448,14 @@ impl Hdf5Writer { /// variable-length string dataset, leaving its extent and every other /// element alone. /// - /// The replacements go into a fresh global heap collection and only the - /// vlen references of the named elements are rewritten, so the cost is the + /// The replacements go into the global heap and only the vlen + /// references of the named elements are rewritten, so the cost is the /// new strings plus the chunks those references live in — not the column. - /// The objects the old references pointed at are freed, so repeated - /// updates reuse space instead of growing the file. This is what libhdf5 + /// The objects the old references pointed at are freed *before* the + /// replacement is allocated, so repeated updates reuse space instead of + /// growing the file — including across close/reopen cycles, where the + /// in-memory free list starts empty and only this free-first order lets + /// the session reuse the block it just released. This is what libhdf5 /// does: `H5T__vlen_disk_write` deletes the reference it read into the /// conversion background buffer before storing the new one. /// @@ -2999,14 +4467,10 @@ impl Hdf5Writer { start: u64, strings: &[&str], ) -> IoResult<()> { - use crate::format::global_heap::{ - encode_vlen_reference, vlen_reference_size, GlobalHeapCollection, - }; + use crate::format::global_heap::{encode_vlen_reference, vlen_reference_size}; use crate::format::messages::datatype::DatatypeMessage; - // An empty batch must not reach the heap write below: an empty - // collection still encodes to the 4096-byte `H5HG_MINALLOC` minimum, - // so the file would grow by a block nothing references. + // An empty batch is a no-op: nothing to replace, nothing to free. if strings.is_empty() { return Ok(()); } @@ -3019,7 +4483,7 @@ impl Hdf5Writer { // Snapshot what the write needs, then drop the guard: `write_slice` // below re-locks the same slot. - let (charset, dims) = { + let (charset, dims, writable) = { let ds = self.ds(ds_index); let m = ds.lock(); let charset = match m.datatype { @@ -3031,9 +4495,23 @@ impl Hdf5Writer { )) } }; - (charset, m.dataspace.dims.clone()) + let writable = m.chunked.is_some() + || m.fixed_array.is_some() + || m.btree_v2.is_some() + || m.data_addr != UNDEF_ADDR; + (charset, m.dataspace.dims.clone(), writable) }; + // `write_slice_inner` rejects a dataset with neither chunk machinery + // nor allocated data (a reopened dataset whose index was not + // reconstructed) — that rejection must come before the heap write + // below, or every failed call orphans a 4096-byte collection. + if !writable { + return Err(crate::io::IoError::InvalidState( + "dataset has no data allocated".into(), + )); + } + if dims.len() != 1 { return Err(crate::io::IoError::InvalidState(format!( "write_vlen_strings_slice is only for 1-dimension datasets, this one has {}", @@ -3049,12 +4527,6 @@ impl Hdf5Writer { } ensure_vlen_charset(charset, strings)?; - // One collection for the batch, as `append_vlen_strings` does. - let mut gcol = GlobalHeapCollection::new(); - let mut obj_indices = Vec::with_capacity(strings.len()); - for s in strings { - obj_indices.push(gcol.add_object(s.as_bytes().to_vec())?); - } let ref_size = vlen_reference_size(&self.ctx); // Elements the append buffer holds are not in the chunks yet: hand @@ -3069,12 +4541,23 @@ impl Hdf5Writer { // which deletes them before storing the new reference. let superseded = self.current_element_bytes(ds_index, start, end - start, ref_size)?; - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; + // Free the superseded objects *before* allocating the replacement, + // the order `H5T__vlen_disk_write` uses. The freed block satisfies + // the allocation below within this same session, so a reopen-and- + // replace loop keeps the file flat — no persisted free-space + // information exists to carry it across sessions (issue #10). The + // cost, shared with libhdf5: a failure between here and the ref + // write below leaves the dataset's old references dangling. + self.release_vlen_references(&superseded)?; + + // The insert comes after the release above so the space the release + // recovered — a freed block, or in-collection bytes the release just + // listed in `cwfs` — can satisfy this batch. + let items: Vec<&[u8]> = strings.iter().map(|s| s.as_bytes()).collect(); + let placements = self.insert_vlen_objects(&items)?; let mut refs = Vec::with_capacity(strings.len() * ref_size); - for (i, &obj_idx) in obj_indices.iter().enumerate() { + for (i, &(gcol_addr, obj_idx)) in placements.iter().enumerate() { refs.extend_from_slice(&encode_vlen_reference( crate::format::global_heap::vlen_seq_len(strings[i].len())?, gcol_addr, @@ -3085,10 +4568,6 @@ impl Hdf5Writer { self.write_slice_inner(ds_index, &[start], &[strings.len() as u64], &refs)?; - // Only now that the new references are in place: a failure above must - // not leave the file naming objects this already freed. - self.release_vlen_references(&superseded)?; - Ok(()) } @@ -3167,10 +4646,20 @@ impl Hdf5Writer { /// Free the global heap objects `refs` names, so replacing a vlen element /// does not strand what it used to point at. /// + /// Callers pass refs only for *top-level* vlen datatypes (the + /// `collect_refs` / `is_vlen` decisions at the prune, delete and + /// attribute-release sites all match `VarLenString`/`VarLenSequence`). + /// A compound datatype with vlen members — writable only by a foreign + /// library, never by this crate — keeps its members' heap objects when + /// its storage is pruned, deleted or replaced. + /// /// This is libhdf5's `H5HG_remove` reached through `H5T__vlen_disk_delete`: /// the object leaves its collection, the collection is rewritten at its /// existing size with the recovered bytes given to the free-space marker, /// and a collection that ends up empty returns its block to the allocator. + /// A rewritten collection's recovered space is listed in `cwfs` for + /// [`insert_vlen_objects`](Self::insert_vlen_objects) to pack into; a + /// freed block leaves the list. /// A nil reference (address 0 or `UNDEF_ADDR`) names no object. The /// address decides, not the sequence length: this crate's writers store /// even the empty string as a real heap object, so a zero-length reference @@ -3220,6 +4709,12 @@ impl Hdf5Writer { per_collection.entry(addr).or_default().push(idx); } + // The `cwfs` lock is held across the sweep: it serializes these + // collection-block rewrites (and frees) against + // `insert_vlen_objects`, which may be packing new objects into the + // same blocks. + let objhdr = GlobalHeapCollection::object_disk_size(&self.ctx, 0); + let mut cwfs = self.cwfs.lock(); for (addr, indices) in per_collection { // A collection is at least 4096 bytes (H5HG_MINALLOC) and most are // exactly that, so one read usually covers the whole image; only @@ -3244,9 +4739,19 @@ impl Hdf5Writer { } if gcol.is_empty() { self.allocator.free(addr, declared as u64); + // The block is gone; a lingering entry would let an insert + // pack into space the allocator can hand to anything. + cwfs.retain(|e| e.addr != addr); } else { let rewritten = gcol.encode_at_size(&self.ctx, declared)?; self.handle.write_at(addr, &rewritten)?; + // The recovered bytes are packable now — list them, the way + // libhdf5's `H5HG_remove` adds the heap to `cwfs`. + if let Some(free) = gcol.free_space_at(&self.ctx, declared) { + if free >= 2 * objhdr { + cwfs_note(&mut cwfs, addr, declared, free); + } + } } } Ok(()) @@ -3257,15 +4762,7 @@ impl Hdf5Writer { /// The attribute will be written as a message in the dataset's object /// header when the file is finalized. pub fn add_dataset_attribute(&self, ds_index: usize, attr: AttributeMessage) -> IoResult<()> { - let count = self.dataset_count(); - if ds_index >= count { - return Err(crate::io::IoError::InvalidState(format!( - "dataset index {} out of range (have {})", - ds_index, count - ))); - } - self.ds(ds_index).lock().attributes.push(attr); - Ok(()) + self.set_attribute(AttrTarget::Dataset(ds_index), attr) } /// Add (or replace) an attribute on a group identified by its full path. @@ -3274,22 +4771,7 @@ impl Hdf5Writer { /// file is finalized. An existing attribute with the same name is /// replaced, matching [`add_root_attribute`](Self::add_root_attribute). pub fn add_group_attribute(&self, group_path: &str, attr: AttributeMessage) -> IoResult<()> { - for grp in self.group_refs() { - let mut g = grp.lock(); - if g.name == group_path && !g.deleted { - let attrs = &mut g.attributes; - if let Some(pos) = attrs.iter().position(|a| a.name == attr.name) { - attrs[pos] = attr; - } else { - attrs.push(attr); - } - return Ok(()); - } - } - Err(crate::io::IoError::NotFound(format!( - "group '{}' not found", - group_path - ))) + self.set_attribute(AttrTarget::Group(group_path), attr) } /// Build a variable-length UTF-8 string attribute message. @@ -3305,22 +4787,16 @@ impl Hdf5Writer { /// `set_attr_string` value is always stored as a true variable-length /// string rather than the fixed-length string it used to be. /// - /// One global heap collection is allocated per attribute, matching the - /// per-call collection of [`create_vlen_string_dataset`](Self::create_vlen_string_dataset). - /// A single shared attribute heap would avoid the per-attribute padding - /// (`H5HG_MINALLOC` = 4096 bytes) but is a heap-management change that - /// would also need to cover the dataset path. - pub fn vlen_string_attribute(&self, name: &str, value: &str) -> IoResult { - use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + /// The string's heap object is placed by + /// [`insert_vlen_objects`](Self::insert_vlen_objects), so consecutive + /// attributes pack into a shared collection instead of each paying the + /// 4096-byte `H5HG_MINALLOC` minimum for a block that holds one string. + fn vlen_string_attribute(&self, name: &str, value: &str) -> IoResult { + use crate::format::global_heap::encode_vlen_reference; use crate::format::messages::dataspace::DataspaceMessage; use crate::format::messages::datatype::DatatypeMessage; - let mut gcol = GlobalHeapCollection::new(); - let obj_idx = gcol.add_object(value.as_bytes().to_vec())?; - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; - + let (gcol_addr, obj_idx) = self.insert_vlen_objects(&[value.as_bytes()])?[0]; let seq_len = crate::format::global_heap::vlen_seq_len(value.len())?; let data = encode_vlen_reference(seq_len, gcol_addr, obj_idx as u32, &self.ctx); Ok(AttributeMessage { @@ -3343,16 +4819,18 @@ impl Hdf5Writer { /// that shape. /// /// The caller owns the invariant that `values.len()` equals the product of - /// `shape` (the public setters validate it before calling). One global heap - /// collection is allocated for the whole array (all elements share it), - /// matching the per-attribute collection of the scalar path. - pub fn vlen_string_array_attribute( - &mut self, + /// `shape` (the public setters validate it before calling). The element + /// objects are placed by + /// [`insert_vlen_objects`](Self::insert_vlen_objects) — a zero-element + /// array allocates nothing, and each reference carries its element's + /// own collection address. + fn vlen_string_array_attribute( + &self, name: &str, values: &[&str], shape: &[u64], ) -> IoResult { - use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + use crate::format::global_heap::encode_vlen_reference; use crate::format::messages::dataspace::DataspaceMessage; use crate::format::messages::datatype::DatatypeMessage; @@ -3362,23 +4840,15 @@ impl Hdf5Writer { "vlen_string_array_attribute values.len() must equal product(shape)" ); - let mut gcol = GlobalHeapCollection::new(); - // (byte length, heap object index) per element, in order. - let mut entries: Vec<(u32, u16)> = Vec::with_capacity(values.len()); - for v in values { - let obj_idx = gcol.add_object(v.as_bytes().to_vec())?; - entries.push((crate::format::global_heap::vlen_seq_len(v.len())?, obj_idx)); - } - let gcol_encoded = gcol.encode(&self.ctx); - let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); - self.handle.write_at(gcol_addr, &gcol_encoded)?; + let items: Vec<&[u8]> = values.iter().map(|v| v.as_bytes()).collect(); + let placements = self.insert_vlen_objects(&items)?; let mut data = Vec::with_capacity(values.len() * 16); - for (len, obj_idx) in &entries { + for (i, &(gcol_addr, obj_idx)) in placements.iter().enumerate() { data.extend_from_slice(&encode_vlen_reference( - *len, + crate::format::global_heap::vlen_seq_len(values[i].len())?, gcol_addr, - *obj_idx as u32, + obj_idx as u32, &self.ctx, )); } @@ -3910,6 +5380,7 @@ impl Hdf5Writer { pipeline: Option, ) -> IoResult { let create = self.begin_create(name)?; + let name = create.name.as_str(); validate_chunk_geometry(dims, max_dims, chunk_dims)?; if max_dims.contains(&u64::MAX) { return Err(crate::io::IoError::InvalidState( @@ -3986,6 +5457,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: pipeline, deleted: false, + extent_dirty: false, fill_value: None, layout_version, chunked: None, @@ -4055,9 +5527,10 @@ impl Hdf5Writer { chunk_dims: &[u64], pipeline: Option, ) -> IoResult { - use crate::format::chunk_index::btree_v2::{Bt2Header, BT2_NODE_SIZE}; + use crate::format::chunk_index::btree_v2::Bt2Header; let create = self.begin_create(name)?; + let name = create.name.as_str(); validate_chunk_geometry(dims, max_dims, chunk_dims)?; let ndims = dims.len(); let chunk_bytes: u64 = chunk_dims.iter().product::() * datatype.element_size() as u64; @@ -4081,10 +5554,11 @@ impl Hdf5Writer { // wider rank than that has no valid geometry, so reject it here rather // than emit a tree no reader can walk. let record_size = bt2_index.record_size(&self.ctx) as usize; - if (BT2_NODE_SIZE as usize) < 10 + 3 * record_size { + let node_size = bt2_index.node_size as usize; + if node_size < 10 + 3 * record_size { return Err(crate::io::IoError::InvalidState(format!( "a {ndims}-dimension v2 B-tree record is {record_size} bytes, too wide \ - for a {BT2_NODE_SIZE}-byte node" + for a {node_size}-byte node" ))); } @@ -4118,6 +5592,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: pipeline, deleted: false, + extent_dirty: false, fill_value: None, layout_version, chunked: None, @@ -4151,6 +5626,7 @@ impl Hdf5Writer { compression_level: u32, ) -> IoResult { let create = self.begin_create(name)?; + let name = create.name.as_str(); validate_chunk_geometry(dims, max_dims, chunk_dims)?; ensure_unlimited_is_leading(max_dims)?; let element_size = datatype.element_size() as u64; @@ -4222,6 +5698,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: Some(FilterPipeline::deflate(compression_level)), deleted: false, + extent_dirty: false, fill_value: None, layout_version, fixed_array: None, @@ -4257,6 +5734,7 @@ impl Hdf5Writer { pipeline: FilterPipeline, ) -> IoResult { let create = self.begin_create(name)?; + let name = create.name.as_str(); validate_chunk_geometry(dims, max_dims, chunk_dims)?; ensure_unlimited_is_leading(max_dims)?; let element_size = datatype.element_size() as u64; @@ -4323,6 +5801,7 @@ impl Hdf5Writer { obj_header_encoded_size: 0, filter_pipeline: Some(pipeline), deleted: false, + extent_dirty: false, fill_value: None, layout_version, fixed_array: None, @@ -4887,88 +6366,569 @@ impl Hdf5Writer { "extend_dataset cannot shrink dimension {d} from {cur} to {new}" ))); } - // An absent maximum shape means the shape is fixed (libhdf5 - // defaults maxdims to dims at creation), so any growth exceeds it. - match m.dataspace.max_dims { - Some(ref max) if new > max[d] => { - return Err(crate::io::IoError::InvalidState(format!( - "extend_dataset dimension {d} ({new}) exceeds the maximum {}", - max[d] - ))); + // An absent maximum shape means the shape is fixed (libhdf5 + // defaults maxdims to dims at creation), so any growth exceeds it. + match m.dataspace.max_dims { + Some(ref max) if new > max[d] => { + return Err(crate::io::IoError::InvalidState(format!( + "extend_dataset dimension {d} ({new}) exceeds the maximum {}", + max[d] + ))); + } + None if new > cur => { + return Err(crate::io::IoError::InvalidState(format!( + "extend_dataset dimension {d} ({new}) exceeds the maximum {cur}: \ + a dataset without a stored maximum shape is fixed at its extent" + ))); + } + _ => {} + } + } + if m.dataspace.dims != new_dims { + m.dataspace.dims = new_dims.to_vec(); + m.extent_dirty = true; + } + Ok(()) + } + + /// Set the logical extent of a chunked dataset, growing **or shrinking** + /// any dimension (unlike [`extend_dataset`](Self::extend_dataset), which + /// only grows). + /// + /// A shrink prunes the stored chunks the way libhdf5's + /// `H5D__chunk_prune_by_extent` (H5Dchunk.c) does: a chunk entirely + /// beyond the new extent leaves the chunk index and its block is freed + /// for reuse (kept under SWMR, where a live reader may still hold its + /// address — the rule `H5Dearray.c` applies in `idx_remove`), and a + /// chunk the new extent cuts through has its out-of-extent region + /// overwritten with the fill value, so growing the extent back exposes + /// fill values rather than the stale data. + pub fn set_dataset_extent(&self, index: usize, new_dims: &[u64]) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + let old_dims = { + let m = ds.lock(); + let is_unindexed = + m.chunked.is_none() && m.fixed_array.is_none() && m.btree_v2.is_none(); + if is_unindexed { + return Err(crate::io::IoError::InvalidState( + "can only set the extent of chunked datasets".into(), + )); + } + if new_dims.len() != m.dataspace.dims.len() { + return Err(crate::io::IoError::InvalidState(format!( + "set_extent rank mismatch: dataset has {} dimensions, got {}", + m.dataspace.dims.len(), + new_dims.len() + ))); + } + // A shrink can cut into buffered rows, whose recorded base would + // then point past the extent; refuse rather than reconcile. + if m.append.is_some() { + return Err(crate::io::IoError::InvalidState( + "set_extent cannot run while the dataset has buffered appends; \ + flush them first" + .into(), + )); + } + // An absent maximum shape means the shape is fixed (libhdf5 + // defaults maxdims to dims at creation), so growth is bounded by + // the extent. + match m.dataspace.max_dims { + Some(ref max) => { + for (d, (&new, &mx)) in new_dims.iter().zip(max).enumerate() { + if new > mx { + return Err(crate::io::IoError::InvalidState(format!( + "set_extent dimension {d} ({new}) exceeds the maximum {mx}" + ))); + } + } + } + None => { + for (d, (&new, &cur)) in new_dims.iter().zip(&m.dataspace.dims).enumerate() { + if new > cur { + return Err(crate::io::IoError::InvalidState(format!( + "set_extent dimension {d} ({new}) exceeds the maximum {cur}: \ + a dataset without a stored maximum shape is fixed at its extent" + ))); + } + } + } + } + m.dataspace.dims.clone() + }; + // A shrink strands chunks; prune them (and refill the straddlers) + // *before* the dims update — chunk addressing uses the + // maximum-extent grid, which the update does not change, and the + // helpers re-lock the slot themselves. + if new_dims.iter().zip(&old_dims).any(|(&n, &o)| n < o) { + self.prune_chunks_beyond(index, new_dims)?; + } + let mut m = ds.lock(); + if m.dataspace.dims != new_dims { + m.dataspace.dims = new_dims.to_vec(); + m.extent_dirty = true; + } + Ok(()) + } + + /// Remove and refill the chunks a shrink to `new_dims` strands — the + /// libhdf5 `H5D__chunk_prune_by_extent` behavior. A chunk entirely + /// beyond the new extent leaves the index and its block is freed (kept + /// under SWMR, where a live reader may still hold its address); a chunk + /// the extent cuts through gets its out-of-extent region refilled with + /// the fill value, so a later regrow reads fill, not stale elements. + /// + /// Runs *before* the dims update: the index grid chunks are addressed in + /// comes from the maximum extent, which a shrink never changes, so every + /// stored entry still resolves. The caller holds the dataset's op lock. + fn prune_chunks_beyond(&self, index: usize, new_dims: &[u64]) -> IoResult<()> { + let geo = self.chunk_geometry(index)?; + // A vlen dataset's elements are global-heap IDs: the pruned chunks + // still reference live heap objects, so the walkers read each dead + // chunk's bytes before freeing its block and the heap objects are + // released here — otherwise every shrink strands its strings in the + // file. `release_vlen_references` is a SWMR no-op, so the reads are + // skipped under SWMR too. + let collect_refs = !self.swmr_active && { + let ds = self.ds(index); + let m = ds.lock(); + matches!( + m.datatype, + DatatypeMessage::VarLenString { .. } | DatatypeMessage::VarLenSequence { .. } + ) + }; + let (straddlers, dead_refs) = match geo.kind { + ChunkIndexKind::ExtensibleArray => { + self.prune_ea_chunks(index, &geo, new_dims, collect_refs)? + } + ChunkIndexKind::FixedArray => { + self.prune_fa_chunks(index, &geo, new_dims, collect_refs)? + } + ChunkIndexKind::BtreeV2 => { + self.prune_bt2_chunks(index, &geo, new_dims, collect_refs)? + } + }; + if !dead_refs.is_empty() { + self.release_vlen_references(&dead_refs)?; + } + // Whole-chunk read-modify-write per straddler: an unfiltered chunk + // rewrites in place, a filtered one re-places through `place_chunk`. + let chunk_bytes = geo.chunk_bytes() as usize; + for coords in straddlers { + let Some(mut data) = self.read_chunk_at_coords(index, &coords)? else { + continue; + }; + let fill = self.new_chunk_buffer(index, chunk_bytes); + let replaced = refill_chunk_beyond_extent( + &mut data, + &fill, + &coords, + &geo.chunk_dims, + new_dims, + geo.element_size as usize, + ); + // Release before the write-back: a filtered straddler re-places + // its block, and freed heap space must be visible to that + // allocation (free-before-alloc, as everywhere else). + if collect_refs && !replaced.is_empty() { + self.release_vlen_references(&replaced)?; + } + self.write_chunk_at_coords(index, &coords, &data)?; + } + Ok(()) + } + + /// Extensible-array half of [`prune_chunks_beyond`](Self::prune_chunks_beyond): + /// walk every slot the array has ever set, free and clear the entries of + /// chunks entirely beyond `new_dims`, and return the grid coordinates of + /// the chunks that straddle it, plus — when `collect_refs` — the dead + /// chunks' element bytes so the caller can release their heap objects. + fn prune_ea_chunks( + &self, + index: usize, + geo: &ChunkGeometry, + new_dims: &[u64], + collect_refs: bool, + ) -> IoResult<(Vec>, Vec)> { + let ds = self.ds(index); + // One slot guard for the whole walk, the `record_ea_chunk` pattern: + // `self.handle`/`self.allocator`/`self.ctx` are disjoint fields. + let mut m = ds.lock(); + let is_filtered = m.filter_pipeline.is_some(); + let pipeline = m.filter_pipeline.clone(); + let chunk_bytes = geo.chunk_bytes(); + let (ea_geo, max_nelmts_bits, chunk_size_len, max_idx) = { + let c = m.chunked.as_ref().unwrap(); + let p = &c.earray_params; + ( + EaGeometry::new( + p.idx_blk_elmts, + p.data_blk_min_elmts, + p.sup_blk_min_data_ptrs, + p.max_nelmts_bits, + p.max_dblk_page_nelmts_bits, + )?, + p.max_nelmts_bits, + c.chunk_size_len, + c.ea_header.max_idx_set, + ) + }; + + let mut straddlers = Vec::new(); + let mut dead_refs = Vec::new(); + + // The decoded data block the walk is currently inside, written back + // when the walk leaves it (or ends) having cleared an entry. + enum Dblk { + Unfiltered(ExtensibleArrayDataBlock), + Filtered(FilteredDataBlock), + } + let mut cache: Option<(u64, Dblk, bool)> = None; + let flush = |cache: &mut Option<(u64, Dblk, bool)>| -> IoResult<()> { + if let Some((addr, blk, dirty)) = cache.take() { + if dirty { + let enc = match &blk { + Dblk::Unfiltered(d) => d.encode(&self.ctx, max_nelmts_bits), + Dblk::Filtered(d) => d.encode(&self.ctx, max_nelmts_bits, chunk_size_len), + }; + self.handle.write_at(addr, &enc)?; + } + } + Ok(()) + }; + // Consecutive slots resolve through the same super block, so keep + // the last decode. Super blocks are only read here — clearing a + // data-block element never moves the block — so it never dirties. + let mut sblk_cache: Option<(usize, ExtensibleArraySuperBlock)> = None; + + let mut slot = 0u64; + while slot < max_idx { + let coords = crate::io::chunk_grid::coords_of( + &geo.dims, + geo.max_dims.as_deref(), + &geo.chunk_dims, + slot, + )?; + if !chunk_outside_extent(&coords, &geo.chunk_dims, new_dims) { + if chunk_straddles_extent(&coords, &geo.chunk_dims, new_dims) { + straddlers.push(coords); + } + slot += 1; + continue; + } + match ea_geo.locate(slot)? { + EaLoc::Index { elem } => { + let c = m.chunked.as_mut().unwrap(); + if is_filtered { + let fiblk = c.filt_iblk.as_mut().unwrap(); + let e = fiblk.elements[elem]; + if e.addr != UNDEF_ADDR { + if collect_refs { + if let Some(bytes) = self.read_chunk_block( + pipeline.as_ref(), + e.addr, + e.nbytes, + e.filter_mask, + )? { + dead_refs.extend_from_slice(&bytes); + } + } + if !self.swmr_active { + self.allocator.free(e.addr, e.nbytes); + } + fiblk.elements[elem] = FilteredChunkEntry { + addr: UNDEF_ADDR, + nbytes: 0, + filter_mask: 0, + }; + } + } else { + let a = c.ea_iblk.elements[elem]; + if a != UNDEF_ADDR { + if collect_refs { + if let Some(bytes) = + self.read_chunk_block(pipeline.as_ref(), a, chunk_bytes, 0)? + { + dead_refs.extend_from_slice(&bytes); + } + } + if !self.swmr_active { + self.allocator.free(a, chunk_bytes); + } + c.ea_iblk.elements[elem] = UNDEF_ADDR; + } + } + slot += 1; + } + EaLoc::Dblk(l) => { + if l.paged { + return Err(crate::io::IoError::InvalidState(format!( + "chunk index {slot} lives in a paged extensible-array \ + data block, which is not yet supported" + ))); + } + let dblk_start = slot - l.offset_in_dblk; + let dblk_end = dblk_start + l.dblk_nelmts; + // Resolve the data block's address; an undefined super or + // data block means nothing in its whole element range was + // ever written, so the walk skips the range. + let dblk_addr = { + let c = m.chunked.as_ref().unwrap(); + match l.path { + EaDblkPath::Direct { idx } => { + if is_filtered { + c.filt_iblk.as_ref().unwrap().dblk_addrs[idx] + } else { + c.ea_iblk.dblk_addrs[idx] + } + } + EaDblkPath::ViaSblk { + sblk_off, + local_dblk, + ndblks_in_sblk, + .. + } => { + let sblk_addr = if is_filtered { + c.filt_iblk.as_ref().unwrap().sblk_addrs[sblk_off] + } else { + c.ea_iblk.sblk_addrs[sblk_off] + }; + if sblk_addr == UNDEF_ADDR { + UNDEF_ADDR + } else { + if sblk_cache.as_ref().map(|&(o, _)| o) != Some(sblk_off) { + let buf = self.handle.read_at_most(sblk_addr, 65536)?; + let sb = ExtensibleArraySuperBlock::decode( + &buf, + &self.ctx, + max_nelmts_bits, + ndblks_in_sblk, + 0, + )?; + sblk_cache = Some((sblk_off, sb)); + } + sblk_cache.as_ref().unwrap().1.dblk_addrs[local_dblk] + } + } + } + }; + if dblk_addr == UNDEF_ADDR { + slot = dblk_end; + continue; + } + if cache.as_ref().map(|&(a, _, _)| a) != Some(dblk_addr) { + flush(&mut cache)?; + let buf = self.handle.read_at_most(dblk_addr, 65536)?; + let blk = if is_filtered { + Dblk::Filtered(FilteredDataBlock::decode( + &buf, + &self.ctx, + max_nelmts_bits, + l.dblk_nelmts as usize, + chunk_size_len, + )?) + } else { + Dblk::Unfiltered(ExtensibleArrayDataBlock::decode( + &buf, + &self.ctx, + max_nelmts_bits, + l.dblk_nelmts as usize, + )?) + }; + cache = Some((dblk_addr, blk, false)); + } + let (_, blk, dirty) = cache.as_mut().unwrap(); + match blk { + Dblk::Filtered(d) => { + let e = d.elements[l.offset_in_dblk as usize]; + if e.addr != UNDEF_ADDR { + if collect_refs { + if let Some(bytes) = self.read_chunk_block( + pipeline.as_ref(), + e.addr, + e.nbytes, + e.filter_mask, + )? { + dead_refs.extend_from_slice(&bytes); + } + } + if !self.swmr_active { + self.allocator.free(e.addr, e.nbytes); + } + d.elements[l.offset_in_dblk as usize] = FilteredChunkEntry { + addr: UNDEF_ADDR, + nbytes: 0, + filter_mask: 0, + }; + *dirty = true; + } + } + Dblk::Unfiltered(d) => { + let a = d.elements[l.offset_in_dblk as usize]; + if a != UNDEF_ADDR { + if collect_refs { + if let Some(bytes) = + self.read_chunk_block(pipeline.as_ref(), a, chunk_bytes, 0)? + { + dead_refs.extend_from_slice(&bytes); + } + } + if !self.swmr_active { + self.allocator.free(a, chunk_bytes); + } + d.elements[l.offset_in_dblk as usize] = UNDEF_ADDR; + *dirty = true; + } + } + } + slot += 1; + } + } + } + flush(&mut cache)?; + Ok((straddlers, dead_refs)) + } + + /// Fixed-array half of [`prune_chunks_beyond`](Self::prune_chunks_beyond): + /// the whole element array is in memory and flushed at close, so + /// clearing an entry is pure bookkeeping. + fn prune_fa_chunks( + &self, + index: usize, + geo: &ChunkGeometry, + new_dims: &[u64], + collect_refs: bool, + ) -> IoResult<(Vec>, Vec)> { + let ds = self.ds(index); + let mut m = ds.lock(); + let is_filtered = m.filter_pipeline.is_some(); + let pipeline = m.filter_pipeline.clone(); + let chunk_bytes = geo.chunk_bytes(); + let mut straddlers = Vec::new(); + let mut dead_refs = Vec::new(); + let fa = m.fixed_array.as_mut().unwrap(); + let nslots = if is_filtered { + fa.fa_dblk.filtered_elements.len() + } else { + fa.fa_dblk.elements.len() + }; + for lidx in 0..nslots { + let (addr, stored, mask) = if is_filtered { + let e = &fa.fa_dblk.filtered_elements[lidx]; + (e.address, e.chunk_size, e.filter_mask) + } else { + (fa.fa_dblk.elements[lidx], chunk_bytes, 0) + }; + if addr == UNDEF_ADDR { + continue; + } + let coords = crate::io::chunk_grid::coords_of( + &geo.dims, + geo.max_dims.as_deref(), + &geo.chunk_dims, + lidx as u64, + )?; + if chunk_outside_extent(&coords, &geo.chunk_dims, new_dims) { + if collect_refs { + if let Some(bytes) = + self.read_chunk_block(pipeline.as_ref(), addr, stored, mask)? + { + dead_refs.extend_from_slice(&bytes); + } } - None if new > cur => { - return Err(crate::io::IoError::InvalidState(format!( - "extend_dataset dimension {d} ({new}) exceeds the maximum {cur}: \ - a dataset without a stored maximum shape is fixed at its extent" - ))); + if !self.swmr_active { + self.allocator.free(addr, stored); } - _ => {} + if is_filtered { + fa.fa_dblk.filtered_elements[lidx] = FixedArrayFilteredChunkElement { + address: UNDEF_ADDR, + chunk_size: 0, + filter_mask: 0, + }; + } else { + fa.fa_dblk.elements[lidx] = UNDEF_ADDR; + } + } else if chunk_straddles_extent(&coords, &geo.chunk_dims, new_dims) { + straddlers.push(coords); } } - m.dataspace.dims = new_dims.to_vec(); - Ok(()) + Ok((straddlers, dead_refs)) } - /// Set the logical extent of a chunked dataset, growing **or shrinking** - /// any dimension (unlike [`extend_dataset`](Self::extend_dataset), which - /// only grows). - /// - /// Shrinking sets the logical dataspace only: chunks (or parts of - /// chunks) beyond the new extent stay in the file but are no longer - /// visible on read, exactly as libhdf5's `H5Dset_extent` behaves. This - /// is how a partial multi-frame chunk's over-extended frame count is - /// corrected back to the true number of frames written. - pub fn set_dataset_extent(&self, index: usize, new_dims: &[u64]) -> IoResult<()> { + /// V2-B-tree half of [`prune_chunks_beyond`](Self::prune_chunks_beyond): + /// drop the records of chunks beyond the extent — the next flush + /// re-serializes the smaller tree over the node pool and releases the + /// surplus node blocks. + fn prune_bt2_chunks( + &self, + index: usize, + geo: &ChunkGeometry, + new_dims: &[u64], + collect_refs: bool, + ) -> IoResult<(Vec>, Vec)> { let ds = self.ds(index); - let _op = ds.op.lock(); let mut m = ds.lock(); - let is_unindexed = m.chunked.is_none() && m.fixed_array.is_none() && m.btree_v2.is_none(); - if is_unindexed { - return Err(crate::io::IoError::InvalidState( - "can only set the extent of chunked datasets".into(), - )); - } - if new_dims.len() != m.dataspace.dims.len() { - return Err(crate::io::IoError::InvalidState(format!( - "set_extent rank mismatch: dataset has {} dimensions, got {}", - m.dataspace.dims.len(), - new_dims.len() - ))); - } - // A shrink can cut into buffered rows, whose recorded base would - // then point past the extent; refuse rather than reconcile. - if m.append.is_some() { - return Err(crate::io::IoError::InvalidState( - "set_extent cannot run while the dataset has buffered appends; \ - flush them first" - .into(), - )); - } - // An absent maximum shape means the shape is fixed (libhdf5 defaults - // maxdims to dims at creation), so growth is bounded by the extent. - match m.dataspace.max_dims { - Some(ref max) => { - for (d, (&new, &mx)) in new_dims.iter().zip(max).enumerate() { - if new > mx { - return Err(crate::io::IoError::InvalidState(format!( - "set_extent dimension {d} ({new}) exceeds the maximum {mx}" - ))); + let pipeline = m.filter_pipeline.clone(); + let chunk_bytes = geo.chunk_bytes(); + let swmr = self.swmr_active; + let mut straddlers = Vec::new(); + let mut dead_refs = Vec::new(); + let bt2 = m.btree_v2.as_mut().unwrap(); + if bt2.index.filtered { + let records = std::mem::take(&mut bt2.index.filtered_records); + let mut kept = Vec::with_capacity(records.len()); + for r in records { + if chunk_outside_extent(&r.scaled_offsets, &geo.chunk_dims, new_dims) { + if collect_refs { + if let Some(bytes) = self.read_chunk_block( + pipeline.as_ref(), + r.chunk_address, + r.chunk_size, + r.filter_mask, + )? { + dead_refs.extend_from_slice(&bytes); + } + } + if !swmr { + self.allocator.free(r.chunk_address, r.chunk_size); + } + } else { + if chunk_straddles_extent(&r.scaled_offsets, &geo.chunk_dims, new_dims) { + straddlers.push(r.scaled_offsets.clone()); } + kept.push(r); } } - None => { - for (d, (&new, &cur)) in new_dims.iter().zip(&m.dataspace.dims).enumerate() { - if new > cur { - return Err(crate::io::IoError::InvalidState(format!( - "set_extent dimension {d} ({new}) exceeds the maximum {cur}: \ - a dataset without a stored maximum shape is fixed at its extent" - ))); + bt2.index.filtered_records = kept; + } else { + let records = std::mem::take(&mut bt2.index.records); + let mut kept = Vec::with_capacity(records.len()); + for r in records { + if chunk_outside_extent(&r.scaled_offsets, &geo.chunk_dims, new_dims) { + if collect_refs { + if let Some(bytes) = self.read_chunk_block( + pipeline.as_ref(), + r.chunk_address, + chunk_bytes, + 0, + )? { + dead_refs.extend_from_slice(&bytes); + } + } + if !swmr { + self.allocator.free(r.chunk_address, chunk_bytes); } + } else { + if chunk_straddles_extent(&r.scaled_offsets, &geo.chunk_dims, new_dims) { + straddlers.push(r.scaled_offsets.clone()); + } + kept.push(r); } } + bt2.index.records = kept; } - m.dataspace.dims = new_dims.to_vec(); - Ok(()) + Ok((straddlers, dead_refs)) } /// Flush a chunked dataset's index structures to disk (durable). @@ -5191,15 +7151,20 @@ impl Hdf5Writer { let is_indexed = { let ds = self.ds(i); let m = ds.lock(); - m.chunked.is_some() || m.fixed_array.is_some() || m.btree_v2.is_some() + !m.deleted + && (m.chunked.is_some() || m.fixed_array.is_some() || m.btree_v2.is_some()) }; if is_indexed { self.flush_dataset(i)?; } } - // 1. Write each dataset's object header. + // 1. Write each dataset's object header (none for a dataset deleted + // before start_swmr — its storage was freed at delete time). for i in 0..self.dataset_count() { + if self.ds(i).lock().deleted { + continue; + } let ds_header = self.build_dataset_header(i); let encoded = ds_header.encode(); let encoded_size = encoded.len(); @@ -5217,10 +7182,16 @@ impl Hdf5Writer { // pass (a header's encoded size is independent of the address // values it carries) and the content is written in a second. for gi in 0..self.group_count() { + if self.grp(gi).lock().deleted { + continue; + } let size = self.build_group_header(gi).encode().len() as u64; self.grp(gi).lock().obj_header_addr = self.allocator.allocate(size); } for gi in 0..self.group_count() { + if self.grp(gi).lock().deleted { + continue; + } let encoded = self.build_group_header(gi).encode(); let addr = self.grp(gi).lock().obj_header_addr; self.handle.write_at(addr, &encoded)?; @@ -5256,6 +7227,9 @@ impl Hdf5Writer { /// dataset's fill value (zeros when none is defined). fn flush_append_buffers(&mut self) -> IoResult<()> { for i in 0..self.dataset_count() { + if self.ds(i).lock().deleted { + continue; + } self.flush_append_buffer(i)?; } Ok(()) @@ -5293,8 +7267,14 @@ impl Hdf5Writer { let ds = self.ds(i); { let m = ds.lock(); + if m.deleted { + continue; + } if m.obj_header_written_addr.is_some() { - let modified = m.chunked.as_ref().is_some_and(|c| c.chunks_written > 0); + let modified = m.chunked.as_ref().is_some_and(|c| c.chunks_written > 0) + || m.fixed_array.as_ref().is_some_and(|f| f.chunks_written > 0) + || m.btree_v2.as_ref().is_some_and(|b| b.chunks_written > 0) + || m.extent_dirty; if !modified { continue; } @@ -5308,21 +7288,46 @@ impl Hdf5Writer { self.flush_dataset_synced(i, sync)?; } - // 1. Write each dataset's object header. + // Every header block this finalize supersedes — a reopened root or + // group header, a modified dataset's reopened header — is freed + // before its replacement is allocated, so the rewrite reuses the + // block instead of growing the file on every open/close cycle. + // Never under SWMR: a live reader may be walking the old headers, + // the same rule `release_vlen_references` and `place_chunk` follow. + // Hard links can alias one header under several names; the set keeps + // an aliased block from entering the free list twice. + let mut freed_headers = std::collections::HashSet::new(); + + // 1. Write each dataset's object header (deleted datasets get none — + // their storage was already freed at delete time). for i in 0..self.dataset_count() { let ds = self.ds(i); { let mut m = ds.lock(); + if m.deleted { + continue; + } if m.obj_header_written_addr.is_some() { // Existing dataset from append mode. - // If it has chunked info with chunks_written > 0, it was modified + // If any chunk index took writes this session — or its + // extent changed without a chunk write — it was modified // and needs a new object header. - let modified = m.chunked.as_ref().is_some_and(|c| c.chunks_written > 0); + let modified = m.chunked.as_ref().is_some_and(|c| c.chunks_written > 0) + || m.fixed_array.as_ref().is_some_and(|f| f.chunks_written > 0) + || m.btree_v2.as_ref().is_some_and(|b| b.chunks_written > 0) + || m.extent_dirty; if !modified { // Keep the original object header address for the root group link. m.obj_header_addr = m.obj_header_written_addr.unwrap(); continue; } + if !self.swmr_active && m.obj_header_encoded_size > 0 { + let old = m.obj_header_written_addr.take().unwrap(); + if freed_headers.insert(old) { + self.allocator.free(old, m.obj_header_encoded_size as u64); + } + m.obj_header_encoded_size = 0; + } } } let ds_header = self.build_dataset_header(i); @@ -5336,17 +7341,44 @@ impl Hdf5Writer { // header is written later, so addresses are assigned in a first // pass (a header's encoded size is independent of the address // values it carries) and the content is written in a second. + if !self.swmr_active { + for gi in 0..self.group_count() { + let grp = self.grp(gi); + let mut g = grp.lock(); + if g.obj_header_encoded_size > 0 { + if let Some(old) = g.obj_header_written_addr.take() { + if freed_headers.insert(old) { + self.allocator.free(old, g.obj_header_encoded_size as u64); + } + g.obj_header_encoded_size = 0; + } + } + } + } for gi in 0..self.group_count() { + if self.grp(gi).lock().deleted { + continue; + } let size = self.build_group_header(gi).encode().len() as u64; self.grp(gi).lock().obj_header_addr = self.allocator.allocate(size); } for gi in 0..self.group_count() { + if self.grp(gi).lock().deleted { + continue; + } let encoded = self.build_group_header(gi).encode(); let addr = self.grp(gi).lock().obj_header_addr; self.handle.write_at(addr, &encoded)?; } // 2. Write root group object header. + if !self.swmr_active { + if let Some((addr, len)) = self.superseded_root_header.take() { + if freed_headers.insert(addr) { + self.allocator.free(addr, len); + } + } + } let root_header = self.build_root_group_header(); let root_encoded = root_header.encode(); let root_addr = self.allocator.allocate(root_encoded.len() as u64); @@ -5433,6 +7465,11 @@ impl Hdf5Writer { DataLayoutMessage::chunked_v4_btree_v2( m.layout_version, layout_dims, + crate::format::messages::data_layout::Bt2Params { + node_size: bt2.index.node_size, + split_percent: bt2.index.split_percent, + merge_percent: bt2.index.merge_percent, + }, bt2.bt2_header_addr, ) } else { @@ -5791,6 +7828,265 @@ mod tests { std::fs::remove_file(&path).ok(); } + /// The CWFS second pass (`H5F_cwfs_find_free_heap`): an object too big + /// for the listed collection's remaining free space extends the + /// collection in place — the file allocation grows off the end of the + /// file (`H5MF_try_extend`) and the collection's declared size and + /// free-space marker grow with it (`H5HG_extend`) — instead of opening + /// a second collection. + #[test] + fn an_oversized_vlen_insert_extends_the_listed_collection() { + use crate::format::global_heap::GlobalHeapCollection; + + let path = temp_path("cwfs_extend_tail"); + let writer = Hdf5Writer::create(&path).unwrap(); + // A small object opens a minimum-size (4096) listed collection — + // the file's last allocation, so the extension grows the file end. + let p1 = writer.insert_vlen_objects(&[b"hello".as_slice()]).unwrap(); + let big = vec![0x41u8; 5000]; // more than the ~4 KiB remaining + let p2 = writer.insert_vlen_objects(&[big.as_slice()]).unwrap(); + assert_eq!( + p2[0].0, p1[0].0, + "the big object opened a second collection" + ); + + // The block on disk is one grown collection holding both objects. + let img = writer.handle.read_at_most(p1[0].0, 65536).unwrap(); + let (gcol, csize) = GlobalHeapCollection::decode(&img, &writer.ctx).unwrap(); + assert!(csize > 4096, "declared size did not grow: {csize}"); + assert_eq!(gcol.objects.len(), 2); + assert_eq!(gcol.objects[1].data, big); + + writer.close().unwrap(); + let bytes = std::fs::read(&path).unwrap(); + assert_eq!( + bytes.windows(4).filter(|w| *w == b"GCOL").count(), + 1, + "a second collection signature is in the file" + ); + std::fs::remove_file(&path).ok(); + } + + /// The non-tail counterpart: the collection is pinned away from the end + /// of the file, but a released block starts right after it, so the + /// extension consumes the front of that block (`H5MF_try_extend`'s + /// free-section path) and the remainder stays reusable. + #[test] + fn extension_consumes_a_freed_block_after_the_collection() { + use crate::format::global_heap::GlobalHeapCollection; + + let path = temp_path("cwfs_extend_freed"); + let writer = Hdf5Writer::create(&path).unwrap(); + let p1 = writer.insert_vlen_objects(&[b"hello".as_slice()]).unwrap(); + let addr = p1[0].0; + // Land a block right after the collection, pin the file end past + // it, then release it: extension must use the released space. + let spacer = writer.allocator.allocate(8192); + assert_eq!(spacer, addr + 4096, "spacer not adjacent; layout changed"); + writer.allocator.allocate(8); + writer.allocator.free(spacer, 8192); + + let big = vec![0x42u8; 5000]; + let p2 = writer.insert_vlen_objects(&[big.as_slice()]).unwrap(); + assert_eq!(p2[0].0, addr, "the big object opened a second collection"); + + let img = writer.handle.read_at_most(addr, 65536).unwrap(); + let (gcol, csize) = GlobalHeapCollection::decode(&img, &writer.ctx).unwrap(); + assert_eq!(csize, 8192, "grew by max(size, shortfall) = 4096"); + assert_eq!(gcol.objects.len(), 2); + + // The remainder of the released block is still allocatable. + assert_eq!( + writer.allocator.allocate(4096), + addr + 8192, + "the freed block's tail was lost" + ); + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// Issue #10: a reopen-and-replace loop on a vlen string must not grow + /// the file. The superseded heap objects are freed *before* the + /// replacement is allocated, so each session reuses the block it just + /// released even though the free list starts empty on reopen. The old + /// free-after-alloc order failed this by one collection per session. + #[test] + fn vlen_replace_across_reopen_keeps_the_file_flat() { + let path = temp_path("vlen_reopen_flat"); + let payload_a = "a".repeat(64 * 1024); + let payload_b = "b".repeat(64 * 1024); + + let writer = Hdf5Writer::create(&path).unwrap(); + writer + .create_vlen_string_dataset("notes", &["initial"]) + .unwrap(); + writer.close().unwrap(); + + let mut sizes = Vec::new(); + for i in 0..8 { + let writer = Hdf5Writer::open_append(&path).unwrap(); + let payload = if i % 2 == 0 { &payload_a } else { &payload_b }; + writer + .write_vlen_strings_slice(0, 0, &[payload.as_str()]) + .unwrap(); + writer.close().unwrap(); + sizes.push(std::fs::metadata(&path).unwrap().len()); + } + // The first replacement grows the file once (the initial collection + // cannot hold 64 KiB); every later equal-size replacement must land + // in the block its own session just freed. + assert_eq!(&sizes[1..], &vec![sizes[0]; 7][..], "sizes: {sizes:?}"); + + // The reused blocks still form a valid file holding the last value. + let mut reader = Hdf5Reader::open(&path).unwrap(); + assert_eq!( + reader.read_vlen_strings("notes").unwrap(), + vec![payload_b.clone()] + ); + + std::fs::remove_file(&path).ok(); + } + + /// Replacing a vlen string attribute must release the superseded + /// global-heap collection *before* the replacement's collection is + /// allocated, so a reopen-replace loop lands each new value in the block + /// it just freed instead of growing the file by one collection per + /// session — the attribute counterpart of + /// [`vlen_replace_across_reopen_keeps_the_file_flat`]. + #[test] + fn vlen_attr_replace_across_reopen_keeps_the_file_flat() { + let path = temp_path("vlen_attr_reopen_flat"); + let payload_a = "a".repeat(8 * 1024); + let payload_b = "b".repeat(8 * 1024); + + let writer = Hdf5Writer::create(&path).unwrap(); + writer + .set_vlen_string_attribute(AttrTarget::Root, "note", &payload_a) + .unwrap(); + writer.close().unwrap(); + + let mut sizes = Vec::new(); + for i in 0..8 { + let writer = Hdf5Writer::open_append(&path).unwrap(); + let payload = if i % 2 == 0 { &payload_b } else { &payload_a }; + writer + .set_vlen_string_attribute(AttrTarget::Root, "note", payload) + .unwrap(); + writer.close().unwrap(); + sizes.push(std::fs::metadata(&path).unwrap().len()); + } + assert_eq!(&sizes[1..], &vec![sizes[0]; 7][..], "sizes: {sizes:?}"); + + // The reused blocks still hold the last value. + let reader = Hdf5Reader::open(&path).unwrap(); + let attr = reader.root_attr("note").unwrap().clone(); + let mut reader = reader; + assert_eq!(reader.attr_string_value(&attr).unwrap(), payload_a); + + std::fs::remove_file(&path).ok(); + } + + /// A numeric attribute replacing a vlen one goes through the same list + /// owner, so the superseded collection is released even though the new + /// value holds no heap reference: a later same-size vlen attribute must + /// land in the freed block, making the file exactly as large as one that + /// never stored the replaced value. + #[test] + fn numeric_replacing_a_vlen_attr_releases_its_collection() { + let payload = "x".repeat(8 * 1024); + let numeric = || { + AttributeMessage::scalar_numeric( + "x", + DatatypeMessage::i32_type(), + 7i32.to_le_bytes().to_vec(), + ) + }; + + let path_a = temp_path("vlen_attr_cross_a"); + let writer = Hdf5Writer::create(&path_a).unwrap(); + writer + .set_vlen_string_attribute(AttrTarget::Root, "x", &payload) + .unwrap(); + writer.add_root_attribute(numeric()).unwrap(); + writer + .set_vlen_string_attribute(AttrTarget::Root, "y", &payload) + .unwrap(); + writer.close().unwrap(); + + // The same end state written without the replaced vlen value. + let path_b = temp_path("vlen_attr_cross_b"); + let writer = Hdf5Writer::create(&path_b).unwrap(); + writer.add_root_attribute(numeric()).unwrap(); + writer + .set_vlen_string_attribute(AttrTarget::Root, "y", &payload) + .unwrap(); + writer.close().unwrap(); + + assert_eq!( + std::fs::metadata(&path_a).unwrap().len(), + std::fs::metadata(&path_b).unwrap().len() + ); + + let reader = Hdf5Reader::open(&path_a).unwrap(); + let y = reader.root_attr("y").unwrap().clone(); + let mut reader = reader; + assert_eq!(reader.attr_string_value(&y).unwrap(), payload); + + std::fs::remove_file(&path_a).ok(); + std::fs::remove_file(&path_b).ok(); + } + + /// Reopen/write/close cycles must not leak the object-header blocks + /// finalize rewrites: the reopened root header, the reopened group + /// header, and the modified chunked dataset's header are each freed + /// before their replacements are allocated. The chunk rewrite itself is + /// in place (unfiltered chunks never move), so a leak of any header + /// block shows up as monotonic growth here. + #[test] + fn reopen_cycles_reuse_superseded_header_blocks() { + let path = temp_path("header_reuse"); + { + let writer = Hdf5Writer::create(&path).unwrap(); + writer.create_group("/", "g").unwrap(); + let idx = writer + .create_chunked_dataset( + "g/data", + DatatypeMessage::i32_type(), + &[4], + &[u64::MAX], + &[4], + ) + .unwrap(); + let seed: Vec = [1i32, 2, 3, 4] + .iter() + .flat_map(|v| v.to_le_bytes()) + .collect(); + writer.write_chunk(idx, 0, &seed).unwrap(); + writer.close().unwrap(); + } + + let mut sizes = Vec::new(); + for i in 0..6i32 { + let writer = Hdf5Writer::open_append(&path).unwrap(); + let data: Vec = [i; 4].iter().flat_map(|v| v.to_le_bytes()).collect(); + writer.write_chunk(0, 0, &data).unwrap(); + writer.close().unwrap(); + sizes.push(std::fs::metadata(&path).unwrap().len()); + } + assert_eq!(&sizes[1..], &vec![sizes[0]; 5][..], "sizes: {sizes:?}"); + + // The reused header blocks still form a valid file. + let mut reader = Hdf5Reader::open(&path).unwrap(); + let raw = reader.read_dataset_raw("g/data").unwrap(); + let values: Vec = raw + .chunks(4) + .map(|c| i32::from_le_bytes(c.try_into().unwrap())) + .collect(); + assert_eq!(values, vec![5, 5, 5, 5]); + + std::fs::remove_file(&path).ok(); + } + #[test] fn create_empty_file() { let path = temp_path("empty"); @@ -6097,6 +8393,66 @@ mod tests { assert_eq!(recovered, dblk.elements); } + #[test] + fn fixed_array_paged_decode_roundtrip_with_uninitialized_page() { + let ctx = FormatContext { + sizeof_addr: 8, + sizeof_size: 8, + }; + let hdr = FixedArrayHeader::new_for_chunks(&ctx, 2500); + let npages = hdr.npages() as usize; // 3 + let page = hdr.dblk_page_nelmts() as usize; // 1024 + + // Populate pages 0 and 2; leave page 1 entirely undefined so its + // bitmap bit stays clear on encode. + let mut dblk = FixedArrayDataBlock::new_unfiltered(0x1000, 2500); + for i in (0..page).chain(2 * page..2500) { + dblk.elements[i] = 0x10000 + (i as u64) * 0x100; + } + + let mut encoded = encode_fixed_array_dblk(&ctx, &hdr, &dblk); + let prefix = FixedArrayPagedPrefix::decode(&encoded, &ctx, npages as u64).unwrap(); + assert!(prefix.page_initialized(0)); + assert!(!prefix.page_initialized(1)); + assert!(prefix.page_initialized(2)); + + // Corrupt the uninitialized page's bytes the way libhdf5 leaves + // them: arbitrary, no valid checksum. Decode must not look at it. + let page_stride = page * 8 + 4; + let p1 = prefix.prefix_size + page_stride; + for b in &mut encoded[p1..p1 + page_stride] { + *b = 0x5A; + } + + let decoded = decode_fixed_array_dblk(&ctx, &hdr, &encoded, 0).unwrap(); + assert_eq!(decoded.elements, dblk.elements); + assert_eq!(decoded.header_addr, 0x1000); + } + + #[test] + fn fixed_array_paged_decode_filtered_roundtrip() { + let ctx = FormatContext { + sizeof_addr: 8, + sizeof_size: 8, + }; + let chunk_size_len = 4usize; + let hdr = FixedArrayHeader::new_for_filtered_chunks(&ctx, 1500, chunk_size_len as u8); + assert!(hdr.is_paged()); + + let mut dblk = FixedArrayDataBlock::new_filtered(0x2000, 1500); + for (i, e) in dblk.filtered_elements.iter_mut().enumerate() { + e.address = 0x8000 + (i as u64) * 0x40; + e.chunk_size = 100 + i as u64; + e.filter_mask = (i % 3) as u32; + } + + let encoded = encode_fixed_array_dblk(&ctx, &hdr, &dblk); + assert_eq!(encoded.len() as u64, fixed_array_dblk_disk_size(&ctx, &hdr)); + let decoded = decode_fixed_array_dblk(&ctx, &hdr, &encoded, chunk_size_len).unwrap(); + assert_eq!(decoded.filtered_elements, dblk.filtered_elements); + assert_eq!(decoded.client_id, FA_CLIENT_FILT_CHUNK); + } + #[test] fn create_fixed_array_paged_dataset_roundtrip() { let path = temp_path("fixed_array_paged"); @@ -6491,6 +8847,79 @@ mod tests { std::fs::remove_file(&path).ok(); } + /// A v2 B-tree whose header declares a non-default node size — libhdf5 + /// built with a different `H5D_BT2_NODE_SIZE`, or any other writer — + /// reopens for append: the reconstruction adopts the header's node_size, + /// split and merge instead of refusing everything but 2048, and the next + /// flush re-serializes at that size (upstream allocates every node at + /// `hdr->node_size`, H5B2leaf.c / H5B2internal.c). + #[test] + fn a_btree_v2_with_a_foreign_node_size_reopens_and_grows() { + let path = temp_path("bt2_foreign_node_size"); + { + let writer = Hdf5Writer::create(&path).unwrap(); + let idx = writer + .create_btree_v2_dataset( + "data", + DatatypeMessage::f64_type(), + &[0, 0], + &[u64::MAX, u64::MAX], + &[1, 1], + ) + .unwrap(); + // Act as a foreign writer: 512-byte nodes, non-default tuning. + // record_size 24 => a 512-byte leaf holds 20 records, so 85 + // records make a depth-1 tree of 512-byte blocks. + { + let ds = writer.ds(idx); + let mut m = ds.lock(); + let index = &mut m.btree_v2.as_mut().unwrap().index; + index.node_size = 512; + index.split_percent = 90; + index.merge_percent = 30; + } + for i in 0..85u64 { + writer + .write_chunk_btree_v2(idx, &[i, 0], &(i as f64).to_le_bytes()) + .unwrap(); + } + writer.extend_dataset(idx, &[85, 1]).unwrap(); + writer.close().unwrap(); + } + { + let writer = Hdf5Writer::open_append(&path).unwrap(); + let idx = writer.dataset_index("data").unwrap(); + { + let ds = writer.ds(idx); + let m = ds.lock(); + let index = &m.btree_v2.as_ref().unwrap().index; + assert_eq!(index.node_size, 512, "header node_size not adopted"); + assert_eq!(index.split_percent, 90); + assert_eq!(index.merge_percent, 30); + assert_eq!(index.records.len(), 85, "records not walked back"); + } + for i in 85..115u64 { + writer + .write_chunk_btree_v2(idx, &[i, 0], &(i as f64).to_le_bytes()) + .unwrap(); + } + writer.extend_dataset(idx, &[115, 1]).unwrap(); + writer.close().unwrap(); + } + + let mut reader = Hdf5Reader::open(&path).unwrap(); + let raw = reader.read_dataset_raw("data").unwrap(); + let values: Vec = raw + .chunks(8) + .map(|c| f64::from_le_bytes(c.try_into().unwrap())) + .collect(); + assert_eq!(values.len(), 115); + for (i, v) in values.iter().enumerate() { + assert_eq!(*v, i as f64, "element {i}"); + } + std::fs::remove_file(&path).ok(); + } + /// A node's record count falls as well as rises: the tree's first leaf goes /// from a full 84 records to 42 when 85 records force it to split. The node /// image is padded to the whole block so re-serializing overwrites the diff --git a/src/lib.rs b/src/lib.rs index e3fc560..2385e9b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,7 +102,7 @@ pub mod swmr; pub mod types; pub use attribute::{AttrShape, H5Attribute}; -pub use dataset::H5Dataset; +pub use dataset::{H5Dataset, ReadNumeric}; pub use error::{Hdf5Error, Result}; pub use file::{H5File, H5FileOptions}; pub use format::messages::datatype::{ByteOrder, DatatypeMessage}; diff --git a/src/swmr.rs b/src/swmr.rs index f698fdb..acc1f30 100644 --- a/src/swmr.rs +++ b/src/swmr.rs @@ -302,30 +302,28 @@ impl SwmrFileWriter { /// `set_group_attr_string("/entry", "NX_class", "NXentry")`. An existing /// attribute of the same name is replaced. /// - /// The same SWMR visibility rule as [`create_group`](Self::create_group) - /// applies: set before [`start_swmr`](Self::start_swmr) for the attribute - /// to be visible to readers during streaming. + /// Attributes must be set before [`start_swmr`](Self::start_swmr): + /// object headers are frozen while readers stream, so every attribute + /// setter is refused once SWMR is active — libhdf5's rule for SWMR + /// writes too. pub fn set_group_attr_string( &mut self, group_path: &str, name: &str, value: &str, ) -> Result<()> { - let attr = self.inner.writer_mut().vlen_string_attribute(name, value)?; - if group_path == "/" { - self.inner.writer_mut().add_root_attribute(attr); - } else { - self.inner - .writer_mut() - .add_group_attribute(group_path, attr)?; - } + self.inner.writer_mut().set_vlen_string_attribute( + group_attr_target(group_path), + name, + value, + )?; Ok(()) } /// Set a numeric scalar attribute on a group, or on the root group when /// `group_path` is `"/"`. An existing attribute of the same name is - /// replaced. See [`set_group_attr_string`](Self::set_group_attr_string) - /// for the SWMR visibility rule. + /// replaced. Refused after [`start_swmr`](Self::start_swmr) — see + /// [`set_group_attr_string`](Self::set_group_attr_string). pub fn set_group_attr_numeric( &mut self, group_path: &str, @@ -333,13 +331,9 @@ impl SwmrFileWriter { value: &T, ) -> Result<()> { let attr = AttributeMessage::scalar_numeric(name, T::hdf5_type(), scalar_to_bytes(value)); - if group_path == "/" { - self.inner.writer_mut().add_root_attribute(attr); - } else { - self.inner - .writer_mut() - .add_group_attribute(group_path, attr)?; - } + self.inner + .writer_mut() + .set_attribute(group_attr_target(group_path), attr)?; Ok(()) } @@ -390,22 +384,27 @@ impl SwmrFileWriter { /// Set a string attribute on a dataset, addressed by its index. The /// NeXus way to record `units`, `long_name`, `signal`, etc. An existing - /// attribute of the same name is replaced. + /// attribute of the same name is replaced. Refused after + /// [`start_swmr`](Self::start_swmr) — see + /// [`set_group_attr_string`](Self::set_group_attr_string). pub fn set_dataset_attr_string( &mut self, ds_index: usize, name: &str, value: &str, ) -> Result<()> { - let attr = self.inner.writer_mut().vlen_string_attribute(name, value)?; - self.inner - .writer_mut() - .add_dataset_attribute(ds_index, attr)?; + self.inner.writer_mut().set_vlen_string_attribute( + crate::io::writer::AttrTarget::Dataset(ds_index), + name, + value, + )?; Ok(()) } /// Set a numeric scalar attribute on a dataset, addressed by its index. - /// An existing attribute of the same name is replaced. + /// An existing attribute of the same name is replaced. Refused after + /// [`start_swmr`](Self::start_swmr) — see + /// [`set_group_attr_string`](Self::set_group_attr_string). pub fn set_dataset_attr_numeric( &mut self, ds_index: usize, @@ -604,7 +603,15 @@ impl SwmrFileReader { } /// Read a dataset as a typed vector. + /// + /// `T` must have the dataset's exact element width — the same + /// width-checked byte reinterpretation as + /// [`H5Dataset::read_raw`](crate::dataset::H5Dataset::read_raw), with no + /// datatype conversion. Use + /// [`dataset_element_size`](Self::dataset_element_size) to size-check a + /// type at runtime. pub fn read_dataset(&mut self, name: &str) -> Result> { + self.check_element_width::(name)?; bytes_to_typed(self.reader.read_dataset_raw(name)?) } @@ -624,16 +631,33 @@ impl SwmrFileReader { } /// Read a slice (hyperslab) of a dataset as a typed vector. - /// See [`read_slice_raw`](Self::read_slice_raw). + /// See [`read_slice_raw`](Self::read_slice_raw); `T`'s width is checked + /// like [`read_dataset`](Self::read_dataset). pub fn read_slice( &mut self, name: &str, starts: &[u64], counts: &[u64], ) -> Result> { + self.check_element_width::(name)?; bytes_to_typed(self.reader.read_slice(name, starts, counts)?) } + /// The width gate for the typed reads: without it, reinterpreting the + /// raw bytes splits or merges elements — an f64 dataset read as `i32` + /// passed the divisibility check and silently returned twice as many + /// garbage values. + fn check_element_width(&self, name: &str) -> Result<()> { + let stored = self.dataset_element_size(name)?; + if T::element_size() != stored { + return Err(crate::error::Hdf5Error::TypeMismatch(format!( + "read type has element size {} but dataset has element size {stored}", + T::element_size(), + ))); + } + Ok(()) + } + /// Read a variable-length string dataset. pub fn read_vlen_strings(&mut self, name: &str) -> Result> { Ok(self.reader.read_vlen_strings(name)?) @@ -695,6 +719,16 @@ impl SwmrFileReader { } } +/// The writer-side attribute list a group path names: `"/"` is the +/// file-level (root) list, anything else the group's own. +fn group_attr_target(group_path: &str) -> crate::io::writer::AttrTarget<'_> { + if group_path == "/" { + crate::io::writer::AttrTarget::Root + } else { + crate::io::writer::AttrTarget::Group(group_path) + } +} + /// Reinterpret a raw byte buffer as a typed vector. The buffer length must /// be a whole multiple of `T`'s element size. fn bytes_to_typed(raw: Vec) -> Result> { diff --git a/tests/delete_reclamation.rs b/tests/delete_reclamation.rs new file mode 100644 index 0000000..7ed29bd --- /dev/null +++ b/tests/delete_reclamation.rs @@ -0,0 +1,265 @@ +//! Deleting a dataset or group must free the file space it owned — chunk +//! blocks, chunk-index structures, contiguous data, the global-heap +//! objects of variable-length values, and (on reopened files) the on-disk +//! object header — the way libhdf5's `H5O_delete` does. It used to only +//! unlink: every create/delete cycle grew the file by the object's whole +//! footprint, and finalize even wrote a fresh orphan header for each +//! deleted object. +//! +//! The oracle is a settled file size: with reclamation, repeating a +//! create/delete cycle reuses the freed blocks, so many cycles end at the +//! same file size as few. (Freed space is reused, not returned — a byte +//! scan can still see stale block contents, so size is the observable.) + +use std::path::PathBuf; +use std::sync::atomic::{AtomicU64, Ordering}; + +use rust_hdf5::H5File; + +fn unique_tmp(label: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "rust_hdf5_delete_reclaim_{}_{}_{}", + label, + std::process::id(), + n + )); + std::fs::create_dir_all(&dir).unwrap(); + dir.join(format!("{label}.h5")) +} + +fn cleanup(path: &PathBuf) { + let _ = std::fs::remove_file(path); + if let Some(dir) = path.parent() { + let _ = std::fs::remove_dir_all(dir); + } +} + +/// Contiguous vlen dataset: each delete must release the strings' heap +/// objects and the reference block, or every cycle leaks a collection +/// plus the data block. +#[test] +fn deleted_contiguous_vlen_dataset_frees_its_heap_and_data() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("contig_vlen_{cycles}")); + let file = H5File::create(&path).unwrap(); + for _ in 0..cycles { + file.write_vlen_strings("notes", &["alpha", "beta", "gamma"]) + .unwrap(); + file.delete_dataset("notes").unwrap(); + } + // Recreate once more and keep it: the delete must not have broken + // the name for reuse, and the survivor must read back. + file.write_vlen_strings("notes", &["alpha", "beta", "gamma"]) + .unwrap(); + file.close().unwrap(); + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["alpha", "beta", "gamma"] + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(20), size_after(2), "20 delete cycles against 2"); +} + +/// EA-chunked vlen dataset, plain and compressed: the delete walks every +/// chunk, releases its heap objects, frees the chunk blocks, and frees +/// the extensible-array index structures themselves. +#[test] +fn deleted_chunked_vlen_dataset_frees_chunks_heap_and_index() { + let mut variants: Vec<(&str, Option)> = vec![("plain", None)]; + if cfg!(feature = "deflate") { + variants.push(("deflate", Some(rust_hdf5::FilterPipeline::deflate(4)))); + } + for (tag, pipeline) in variants { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("chunked_vlen_{tag}_{cycles}")); + let file = H5File::create(&path).unwrap(); + for c in 0..cycles { + file.create_appendable_vlen_dataset("log", 2, pipeline.clone()) + .unwrap(); + for i in 0..3 { + file.append_vlen_strings("log", &[&format!("a{c}_{i}"), &format!("b{c}_{i}")]) + .unwrap(); + } + file.delete_dataset("log").unwrap(); + } + file.write_vlen_strings("keep", &["survivor"]).unwrap(); + file.close().unwrap(); + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("keep").unwrap().read_vlen_strings().unwrap(), + vec!["survivor"] + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!( + size_after(20), + size_after(2), + "{tag}: 20 delete cycles against 2" + ); + } +} + +/// Fixed-array-indexed dataset (bounded max shape): the delete frees the +/// chunk blocks plus the FA header and data block. +#[test] +fn deleted_fixed_array_dataset_frees_chunks_and_index() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("fa_{cycles}")); + let file = H5File::create(&path).unwrap(); + let row: Vec = (0..4i32).flat_map(|v| v.to_le_bytes()).collect(); + for _ in 0..cycles { + let ds = file + .new_dataset::() + .shape([4usize, 4]) + .chunk(&[1, 4]) + .max_shape(&[Some(8), Some(4)]) + .create("grid") + .unwrap(); + for f in 0..4 { + ds.write_chunk(f, &row).unwrap(); + } + file.delete_dataset("grid").unwrap(); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(20), size_after(2), "20 delete cycles against 2"); +} + +/// v2-B-tree-indexed dataset (two unlimited dimensions): the delete frees +/// the chunk blocks plus the BT2 header (and any flushed node blocks). +#[test] +fn deleted_btree_v2_dataset_frees_chunks_and_header() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("bt2_{cycles}")); + let file = H5File::create(&path).unwrap(); + let tile: Vec = (0..4i32).flat_map(|v| v.to_le_bytes()).collect(); + for _ in 0..cycles { + let ds = file + .new_dataset::() + .shape([4usize, 4]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .create("tiles") + .unwrap(); + for r in 0..2usize { + for c in 0..2usize { + ds.write_chunk_at(&[r, c], &tile).unwrap(); + } + } + file.delete_dataset("tiles").unwrap(); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(20), size_after(2), "20 delete cycles against 2"); +} + +/// Deleting a group releases its attributes' heap objects and every child +/// object's storage. +#[test] +fn deleted_group_frees_attr_heap_and_child_storage() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("group_{cycles}")); + let file = H5File::create(&path).unwrap(); + for _ in 0..cycles { + let g = file.root_group().create_group("run").unwrap(); + g.set_attr_string("NX_class", "NXentry").unwrap(); + g.new_dataset::().shape([64]).create("temp").unwrap(); + file.delete_group("run").unwrap(); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(20), size_after(2), "20 delete cycles against 2"); +} + +/// Freeing the deleted dataset's storage must not touch its neighbors: +/// the survivors' strings and data still read back after a delete in the +/// same session. +#[test] +fn deleting_one_dataset_keeps_the_others_readable() { + let path = unique_tmp("survivors"); + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("keep", &["k0", "k1"]).unwrap(); + file.write_vlen_strings("drop", &["d0", "d1"]).unwrap(); + file.create_appendable_vlen_dataset("log", 2, None).unwrap(); + file.append_vlen_strings("log", &["l0", "l1"]).unwrap(); + file.delete_dataset("drop").unwrap(); + assert!( + file.delete_dataset("drop").is_err(), + "a deleted name must not resolve again" + ); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("keep").unwrap().read_vlen_strings().unwrap(), + vec!["k0", "k1"] + ); + assert_eq!( + file.dataset("log").unwrap().read_vlen_strings().unwrap(), + vec!["l0", "l1"] + ); + assert!(file.dataset("drop").is_err()); + drop(file); + cleanup(&path); +} + +/// A delete in a reopen session frees storage that was parsed back off +/// the disk — the previous session's data blocks, heap objects, and +/// object header block — so delete/recreate sessions settle instead of +/// growing the file every time. +#[test] +fn reopen_session_delete_frees_the_previous_sessions_storage() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("reopen_{cycles}")); + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("notes", &["alpha", "beta"]) + .unwrap(); + file.close().unwrap(); + } + for _ in 0..cycles { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_dataset("notes").unwrap(); + file.write_vlen_strings("notes", &["alpha", "beta"]) + .unwrap(); + file.close().unwrap(); + } + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["alpha", "beta"] + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(10), size_after(2), "10 reopen cycles against 2"); +} diff --git a/tests/group_enumeration.rs b/tests/group_enumeration.rs index 9d0c998..f4b211d 100644 --- a/tests/group_enumeration.rs +++ b/tests/group_enumeration.rs @@ -186,3 +186,99 @@ fn group_names_lists_only_immediate_children() { cleanup(&path); } + +/// A bare `open_rw` + `close` must not change the group structure. The +/// reopen used to rebuild the group registry from dataset paths alone, so +/// a group with no dataset beneath it (empty, subgroup-only, or NeXus +/// attribute-only) vanished at close — and even surviving groups lost +/// their attributes, which the reopen had parsed and then discarded. +#[test] +fn reopen_session_keeps_groups_and_their_attributes() { + let path = unique_tmp("reopen_keeps"); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + root.create_group("empty").unwrap(); + let subgroup_only = root.create_group("subgroup_only").unwrap(); + subgroup_only.create_group("child").unwrap(); + let attr_only = root.create_group("attr_only").unwrap(); + attr_only.set_attr_string("NX_class", "NXdetector").unwrap(); + let with_data = root.create_group("with_data").unwrap(); + with_data.set_attr_string("NX_class", "NXentry").unwrap(); + file.write_vlen_strings("with_data/notes", &["n0"]).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open_rw(&path).unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let root = file.root_group(); + let mut names = root.group_names().unwrap(); + names.sort(); + assert_eq!(names, ["attr_only", "empty", "subgroup_only", "with_data"]); + assert_eq!( + root.group("subgroup_only").unwrap().group_names().unwrap(), + vec!["child".to_string()] + ); + assert_eq!( + root.group("attr_only") + .unwrap() + .attr_string("NX_class") + .unwrap(), + "NXdetector" + ); + assert_eq!( + root.group("with_data") + .unwrap() + .attr_string("NX_class") + .unwrap(), + "NXentry" + ); + assert_eq!( + file.dataset("with_data/notes") + .unwrap() + .read_vlen_strings() + .unwrap(), + vec!["n0"] + ); + drop(file); + cleanup(&path); +} + +/// A reopen session must be able to put an attribute on a group that has +/// no dataset beneath it — the group has to be in the writer's registry +/// for `set_attr_string` to find it. +#[test] +fn reopen_session_can_attribute_a_datasetless_group() { + let path = unique_tmp("reopen_attr_empty"); + + { + let file = H5File::create(&path).unwrap(); + file.root_group().create_group("empty").unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open_rw(&path).unwrap(); + file.root_group() + .group("empty") + .unwrap() + .set_attr_string("NX_class", "NXcollection") + .unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.root_group() + .group("empty") + .unwrap() + .attr_string("NX_class") + .unwrap(), + "NXcollection" + ); + drop(file); + cleanup(&path); +} diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index eb8f5c8..659fc51 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -823,6 +823,130 @@ fn fa_growable_max_shape_readable_by_h5py() { std::fs::remove_file(&path).ok(); } +/// A global-heap collection extended in place (CWFS second pass: bigger +/// declared size, free-space marker moved to the new tail) must stay +/// standard-readable: h5py reads both the object that fit the original +/// 4096 bytes and the one that forced the extension. +#[test] +fn extended_vlen_collection_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("extended_gcol"); + let big = "x".repeat(5000); + { + let file = H5File::create(&path).unwrap(); + let g = file.root_group().create_group("entry").unwrap(); + g.set_attr_string("small", "hello").unwrap(); + g.set_attr_string("big", &big).unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "g = f['entry']\n\ + assert g.attrs['small'] == 'hello', g.attrs['small']\n\ + assert g.attrs['big'] == 'x' * 5000, len(g.attrs['big'])\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// A paged FA index (1200 chunks > 1024 per page) written across two rust +/// sessions — the second reconstructs the paged data block via +/// `open_append` — reads back exactly through h5py. +#[test] +fn fa_paged_reopened_write_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("fa_paged_reopen"); + let n = 1200usize; + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([n]) + .chunk(&[1]) + .max_shape(&[Some(n)]) + .create("wide") + .unwrap(); + // Page 0 only; page 1 (elements 1024..1200) stays uninitialized. + ds.write_slice(&[0], &[600], &(0..600).collect::>()) + .unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + let ds = file.dataset_writer("wide").unwrap(); + ds.write_slice(&[600], &[600], &(600..n as i32).collect::>()) + .unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['wide']\n\ + assert ds.shape == (1200,), ds.shape\n\ + assert ds.chunks == (1,), ds.chunks\n\ + assert np.array_equal(ds[...], np.arange(1200, dtype=np.int32)), ds[...]\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// h5py (libver='latest', fixed shape, 1200 unit chunks) writes a paged FA +/// data block and initializes only page 0; libhdf5 leaves page 1's file +/// space unwritten. Our reader must honor the page-init bitmap: page-0 +/// values read back, page-1 elements read as fill. +#[test] +fn fa_paged_written_by_h5py_readable_by_rust() { + let Some(py) = python() else { return }; + let path = tmp("fa_paged_from_h5py"); + write_with_h5py( + py, + &path, + "name = f.filename; f.close()\n\ + f = h5py.File(name, 'w', libver='latest')\n\ + ds = f.create_dataset('wide', shape=(1200,), chunks=(1,), dtype='().unwrap(); + assert_eq!(vals.len(), 1200); + for (i, v) in vals.iter().enumerate() { + let expect = if i < 600 { i as i32 } else { 0 }; + assert_eq!(*v, expect, "element {i}"); + } + drop(file); + std::fs::remove_file(&path).ok(); +} + +/// The full parity loop: h5py writes the paged FA (page 1 genuinely +/// unwritten by libhdf5), rust `open_append` reconstructs it and writes +/// the remaining chunks, and h5py reads the completed array back. +#[test] +fn fa_paged_written_by_h5py_completed_by_rust() { + let Some(py) = python() else { return }; + let path = tmp("fa_paged_complete"); + write_with_h5py( + py, + &path, + "name = f.filename; f.close()\n\ + f = h5py.File(name, 'w', libver='latest')\n\ + ds = f.create_dataset('wide', shape=(1200,), chunks=(1,), dtype='>()) + .unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['wide']\n\ + assert np.array_equal(ds[...], np.arange(1200, dtype=np.int32)), ds[...]\n", + ); + std::fs::remove_file(&path).ok(); +} + /// Issue #8: `set_libver_latest(true)` writes a version-5 data layout message /// for filtered chunked datasets. Two identical files differing only in the /// knob prove both directions: the default file stays h5py-readable (v4), and @@ -891,3 +1015,176 @@ fn libver_latest_v5_layout_write_and_hdf5_1x_rejection() { std::fs::remove_file(&path_v4).ok(); std::fs::remove_file(&path_v5).ok(); } + +/// Issue #11: datatype-aware conversion reads against an externally-written +/// file — h5py writes int16, big-endian int32, float32, and uint64 datasets; +/// rust-hdf5 converts on read, including the checked-overflow error path. +#[test] +fn numeric_conversion_reads_from_h5py_written_file() { + let Some(py) = python() else { return }; + let path = tmp("numeric_conv"); + write_with_h5py( + py, + &path, + "f.create_dataset('i2', data=np.array([-3, -1, 0, 7], dtype='int16'))\n\ + f.create_dataset('be_i4', data=np.array([-100000, 100000], dtype='>i4'))\n\ + f.create_dataset('f4', data=np.array([1.5, -2.25], dtype='float32'))\n\ + f.create_dataset('u8', data=np.array([1, 2**64 - 1], dtype='uint64'))\n", + ); + + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("i2") + .unwrap() + .read_numeric_as::() + .unwrap(), + vec![-3, -1, 0, 7] + ); + assert_eq!( + file.dataset("be_i4") + .unwrap() + .read_numeric_as::() + .unwrap(), + vec![-100_000, 100_000] + ); + assert_eq!( + file.dataset("f4") + .unwrap() + .read_numeric_as::() + .unwrap(), + vec![1.5, -2.25] + ); + assert_eq!( + file.dataset("u8") + .unwrap() + .read_numeric_as::() + .unwrap(), + vec![1, u128::from(u64::MAX)] + ); + let err = file + .dataset("u8") + .unwrap() + .read_numeric_as::() + .unwrap_err(); + assert!( + err.to_string().contains("does not fit in i64"), + "unexpected error: {err}" + ); + std::fs::remove_file(&path).ok(); +} + +/// `H5Attribute::read_numeric` must accept h5py's standard little-endian +/// numeric attribute datatypes (the strict datatype check cannot be *too* +/// strict), refuse a big-endian one, and `read_numeric_as` must convert it. +#[test] +fn attr_numeric_reads_from_h5py_written_file() { + let Some(py) = python() else { return }; + let path = tmp("attr_numeric"); + write_with_h5py( + py, + &path, + "d = f.create_dataset('d', data=np.zeros(2, dtype='float32'))\n\ + d.attrs.create('i4', np.int32(-7))\n\ + d.attrs.create('f8', np.float64(1.5))\n\ + d.attrs.create('be_i4', np.int32(100000), dtype='>i4')\n", + ); + + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.attr("i4").unwrap().read_numeric::().unwrap(), -7); + assert_eq!(ds.attr("f8").unwrap().read_numeric::().unwrap(), 1.5); + let be = ds.attr("be_i4").unwrap(); + assert!(be.read_numeric::().is_err()); + assert_eq!(be.read_numeric_as::().unwrap(), vec![100_000]); + std::fs::remove_file(&path).ok(); +} + +/// A dataset shrunk with `set_extent` (pruning chunks from the index) and +/// grown back is read by h5py/libhdf5 as retained data plus fill values — +/// the pruned entries must leave the extensible-array index in a state +/// libhdf5 accepts. +#[test] +fn shrunk_and_regrown_dataset_reads_fill_via_h5py() { + let Some(py) = python() else { return }; + let path = tmp("shrink_prune"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([24usize, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(4)]) + .create("data") + .unwrap(); + let vals: Vec = (0..24 * 4).collect(); + ds.write_slice(&[0, 0], &[24, 4], &vals).unwrap(); + ds.set_extent(&[3, 4]).unwrap(); + ds.set_extent(&[24, 4]).unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "d = f['data']\n\ + assert d.shape == (24, 4), d.shape\n\ + v = d[...]\n\ + exp = np.arange(96, dtype='int32').reshape(24, 4)\n\ + assert (v[:3] == exp[:3]).all(), v[:3]\n\ + assert (v[3:] == 0).all(), v[3:]\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// A vlen batch past the 65535-object collection index cap spills into a +/// second collection; libhdf5 must resolve references across both — the +/// per-element collection address is all it needs. +#[test] +fn spilled_vlen_batch_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("vlen_spill"); + { + let file = H5File::create(&path).unwrap(); + let strings = vec!["x"; 65537]; + file.write_vlen_strings("bulk", &strings).unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "d = f['bulk']\n\ + assert d.shape == (65537,), d.shape\n\ + v = d[...]\n\ + assert v[0] == b'x' and v[65535] == b'x' and v[65536] == b'x', v[:3]\n\ + assert (v == b'x').all()\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// Small vlen attributes and datasets packed into one shared collection +/// (the writer's CWFS path) stay readable: h5py resolves each reference +/// by its own (address, index) pair regardless of who else shares the +/// block. +#[test] +fn packed_shared_collection_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("vlen_packed"); + { + let file = H5File::create(&path).unwrap(); + let g = file.root_group().create_group("entry").unwrap(); + g.set_attr_string("NX_class", "NXentry").unwrap(); + g.set_attr_string("title", "packed heap").unwrap(); + file.write_vlen_strings("notes", &["alpha", "beta"]) + .unwrap(); + file.write_vlen_strings("tags", &["red", "green"]).unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "assert f['entry'].attrs['NX_class'] == 'NXentry'\n\ + assert f['entry'].attrs['title'] == 'packed heap'\n\ + assert list(f['notes'][...]) == [b'alpha', b'beta']\n\ + assert list(f['tags'][...]) == [b'red', b'green']\n", + ); + std::fs::remove_file(&path).ok(); +} diff --git a/tests/hard_links.rs b/tests/hard_links.rs index a036d1b..9cf7002 100644 --- a/tests/hard_links.rs +++ b/tests/hard_links.rs @@ -1,9 +1,12 @@ -//! Integration tests for hard-link creation (`H5Group::link`). +//! Integration tests for hard-link creation (`H5Group::link`) and for +//! deletion against hard links (`H5Ldelete` semantics). //! //! A hard link gives an existing object a second name without copying its //! data — the NeXus-style way to expose a dataset at `/entry/data/data` //! while it physically lives elsewhere. Both names must resolve to //! byte-identical data, and the reader must enumerate the aliased path. +//! Deleting a name only unlinks it: the object survives under a +//! remaining hard link, and its storage is freed with the last name. use std::path::PathBuf; use std::sync::atomic::{AtomicU64, Ordering}; @@ -263,6 +266,650 @@ fn swmr_writer_builds_nexus_layout() { cleanup(&path); } +/// Deleting a hard-linked dataset's tree name must not destroy the +/// object: the link is promoted to the primary name, the data stays +/// readable there, and the old path is gone. +#[test] +fn deleting_primary_name_promotes_hard_link() { + let path = unique_tmp("hl_del_promote"); + let data: Vec = (0..8).collect(); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let inst = root.create_group("instrument").unwrap(); + let ds = inst + .new_dataset::() + .shape([8]) + .create("detector") + .unwrap(); + ds.write_raw(&data).unwrap(); + + let data_grp = root.create_group("data").unwrap(); + data_grp.link("detector", "/instrument/detector").unwrap(); + + file.delete_dataset("instrument/detector").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + let survived = file + .dataset("data/detector") + .unwrap() + .read_raw::() + .unwrap(); + assert_eq!(survived, data, "object survives under the hard link"); + assert!( + file.dataset("instrument/detector").is_err(), + "the deleted name must not resolve" + ); + } + + cleanup(&path); +} + +/// Deleting the *last* name frees the storage: cycles of +/// create → link → delete-primary → delete-promoted-name settle to a +/// fixed file size (the oracle of `delete_reclamation.rs`). +#[test] +fn deleting_last_name_frees_storage() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("hl_del_free_{cycles}")); + let vals: Vec = (0..1024).collect(); + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let inst = root.create_group("instrument").unwrap(); + let data_grp = root.create_group("data").unwrap(); + for _ in 0..cycles { + let ds = inst + .new_dataset::() + .shape([1024]) + .create("detector") + .unwrap(); + ds.write_raw(&vals).unwrap(); + data_grp.link("detector", "/instrument/detector").unwrap(); + // First delete only unlinks (the link survives as + // /data/detector); the second removes the last name. + file.delete_dataset("instrument/detector").unwrap(); + file.delete_dataset("data/detector").unwrap(); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(10), size_after(2), "10 cycles against 2"); +} + +/// A hard link from outside the subtree naming an inner *group* keeps +/// that group alive: it is re-homed under the link with its whole +/// subtree renamed, while the rest of the container is deleted. +#[test] +fn delete_group_promotes_outside_linked_inner_group() { + let path = unique_tmp("hl_del_ginner"); + let data: Vec = vec![5, 6, 7]; + let deep_data: Vec = vec![13, 14]; + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let container = root.create_group("container").unwrap(); + let inner = container.create_group("inner").unwrap(); + let ds = inner.new_dataset::().shape([3]).create("ds").unwrap(); + ds.write_raw(&data).unwrap(); + let deep = inner.create_group("deep").unwrap(); + let ds2 = deep.new_dataset::().shape([2]).create("ds2").unwrap(); + ds2.write_raw(&deep_data).unwrap(); + container + .new_dataset::() + .shape([4]) + .create("gone") + .unwrap(); + + root.link("inner_alias", "/container/inner").unwrap(); + + file.delete_group("container").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("inner_alias/ds") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the linked group's dataset must survive under the link" + ); + assert_eq!( + file.dataset("inner_alias/deep/ds2") + .unwrap() + .read_raw::() + .unwrap(), + deep_data, + "the rename must reach the whole promoted subtree" + ); + assert!( + file.dataset("container/inner/ds").is_err(), + "the old path must not resolve" + ); + assert!( + file.dataset("container/gone").is_err(), + "the unlinked sibling must be gone" + ); + } + + cleanup(&path); +} + +/// An outside link naming the deleted group itself turns the delete into +/// a pure rename: the whole subtree survives under the link's path. +#[test] +fn delete_group_promotes_the_group_itself() { + let path = unique_tmp("hl_del_gself"); + let data: Vec = vec![1, 2, 3, 4]; + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let container = root.create_group("container").unwrap(); + let ds = container + .new_dataset::() + .shape([4]) + .create("ds") + .unwrap(); + ds.write_raw(&data).unwrap(); + root.link("keeper", "container").unwrap(); + + file.delete_group("container").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("keeper/ds") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the group must survive under its other name" + ); + assert!( + file.dataset("container/ds").is_err(), + "the deleted name must not resolve" + ); + } + + cleanup(&path); +} + +/// `delete_group` re-homes an inner dataset a hard link from outside +/// still names: the alias survives close, the rest of the subtree is +/// gone. +#[test] +fn delete_group_promotes_outside_linked_dataset() { + let path = unique_tmp("hl_del_rehome"); + let keep: Vec = (0..6).collect(); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let doomed = root.create_group("doomed").unwrap(); + let ds = doomed + .new_dataset::() + .shape([6]) + .create("keep") + .unwrap(); + ds.write_raw(&keep).unwrap(); + doomed + .new_dataset::() + .shape([4]) + .create("gone") + .unwrap(); + + root.link("survivor", "/doomed/keep").unwrap(); + + file.delete_group("doomed").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("survivor").unwrap().read_raw::().unwrap(), + keep, + "outside-linked dataset survives its container" + ); + assert!( + file.dataset("doomed/keep").is_err(), + "old path inside the deleted group must not resolve" + ); + assert!( + file.dataset("doomed/gone").is_err(), + "unlinked sibling must be gone" + ); + } + + cleanup(&path); +} + +/// Deleting a path that names a hard link removes just that link: the +/// target dataset and its tree name are untouched. +#[test] +fn deleting_a_link_path_unlinks_only_the_link() { + let path = unique_tmp("hl_del_link"); + let data: Vec = (0..5).collect(); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let inst = root.create_group("instrument").unwrap(); + let ds = inst + .new_dataset::() + .shape([5]) + .create("detector") + .unwrap(); + ds.write_raw(&data).unwrap(); + let data_grp = root.create_group("data").unwrap(); + data_grp.link("detector", "/instrument/detector").unwrap(); + + file.delete_dataset("data/detector").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("instrument/detector") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the tree name must survive its link's deletion" + ); + assert!( + file.dataset("data/detector").is_err(), + "the deleted link path must not resolve" + ); + } + + cleanup(&path); +} + +/// Deleting a group-link path unlinks just that link: the target group +/// and its tree name stay. +#[test] +fn deleting_a_group_link_path_unlinks_only_the_link() { + let path = unique_tmp("hl_del_glink"); + let data: Vec = vec![21, 22]; + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let container = root.create_group("container").unwrap(); + let inner = container.create_group("inner").unwrap(); + let ds = inner.new_dataset::().shape([2]).create("ds").unwrap(); + ds.write_raw(&data).unwrap(); + root.link("inner_alias", "/container/inner").unwrap(); + + file.delete_group("inner_alias").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("container/inner/ds") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the tree name must survive its link's deletion" + ); + assert!( + file.dataset("inner_alias/ds").is_err(), + "the deleted link path must not resolve" + ); + } + + cleanup(&path); +} + +/// `dataset_writer` resolves a hard-link path to its target, like +/// `H5Dopen`: a write through the alias lands in the one object. +#[test] +fn dataset_writer_resolves_a_hard_link_path() { + let path = unique_tmp("hl_write_alias"); + let data: Vec = (0..4).collect(); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let inst = root.create_group("instrument").unwrap(); + inst.new_dataset::() + .shape([4]) + .chunk(&[2]) + .max_shape(&[None]) + .create("detector") + .unwrap(); + let data_grp = root.create_group("data").unwrap(); + data_grp.link("detector", "/instrument/detector").unwrap(); + + let ds = file.dataset_writer("data/detector").unwrap(); + ds.write_slice(&[0], &[4], &data).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("instrument/detector") + .unwrap() + .read_raw::() + .unwrap(), + data, + "a write through the alias must land in the target" + ); + } + + cleanup(&path); +} + +/// A hard link whose target path is itself a hard link points straight at +/// the object (links have no chain): it survives both other names. +#[test] +fn hard_link_to_a_link_path_targets_the_object() { + let path = unique_tmp("hl_chain"); + let data: Vec = vec![41, 42]; + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let ds = root.new_dataset::().shape([2]).create("ds").unwrap(); + ds.write_raw(&data).unwrap(); + root.link("alias1", "ds").unwrap(); + root.link("alias2", "alias1").unwrap(); + + file.delete_dataset("ds").unwrap(); + file.delete_dataset("alias1").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("alias2").unwrap().read_raw::().unwrap(), + data, + "the last name must keep the object" + ); + } + + cleanup(&path); +} + +/// A reopened file rebuilds hard-link identity: the alias and the tree +/// name are one object again, so deleting one name keeps the data alive +/// under the other. Reopen used to give every alias its own +/// `DatasetInfo` with the same storage addresses — deleting either path +/// freed blocks the other still referenced. +#[test] +fn reopened_file_keeps_hard_link_identity() { + let path = unique_tmp("hl_reopen"); + let data: Vec = (0..10).collect(); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let inst = root.create_group("instrument").unwrap(); + let ds = inst + .new_dataset::() + .shape([10]) + .create("detector") + .unwrap(); + ds.write_raw(&data).unwrap(); + let data_grp = root.create_group("data").unwrap(); + data_grp.link("detector", "/instrument/detector").unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_dataset("instrument/detector").unwrap(); + // Anything the delete wrongly freed gets reused here with other + // bytes — under the old per-alias DatasetInfo reopen, this filler + // landed in the object's storage and the alias read it back. + let filler: Vec = (100..110).collect(); + let f = file + .root_group() + .new_dataset::() + .shape([10]) + .create("filler") + .unwrap(); + f.write_raw(&filler).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("data/detector") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the other name must keep the object across sessions" + ); + assert!( + file.dataset("instrument/detector").is_err(), + "the deleted name must not resolve" + ); + } + + cleanup(&path); +} + +/// Reopening and closing without changes keeps both names resolving. +#[test] +fn reopen_close_preserves_hard_links() { + let path = unique_tmp("hl_reopen_noop"); + let data: Vec = vec![9, 8, 7]; + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let ds = root.new_dataset::().shape([3]).create("ds").unwrap(); + ds.write_raw(&data).unwrap(); + root.link("alias", "ds").unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!(file.dataset("ds").unwrap().read_raw::().unwrap(), data); + assert_eq!( + file.dataset("alias").unwrap().read_raw::().unwrap(), + data + ); + } + + cleanup(&path); +} + +/// Cross-session last-name deletes settle the file size: cycles of +/// reopen → delete both names → recreate dataset + link do not grow the +/// file, so the freed storage is really recovered. +#[test] +fn reopen_delete_both_names_settles_file_size() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("hl_reopen_free_{cycles}")); + let vals: Vec = (0..256).collect(); + let create = |file: &H5File| { + let root = file.root_group(); + let ds = root.new_dataset::().shape([256]).create("ds").unwrap(); + ds.write_raw(&vals).unwrap(); + root.link("alias", "ds").unwrap(); + }; + { + let file = H5File::create(&path).unwrap(); + create(&file); + file.close().unwrap(); + } + for _ in 0..cycles { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_dataset("ds").unwrap(); + file.delete_dataset("alias").unwrap(); + create(&file); + file.close().unwrap(); + } + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("alias").unwrap().read_raw::().unwrap(), + vals + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(10), size_after(2), "10 reopen cycles against 2"); +} + +/// A hard link to a *group* survives reopen too: deleting the container +/// in the second session still re-homes the linked inner group. +#[test] +fn reopened_group_link_promotes_inner_group() { + let path = unique_tmp("hl_reopen_group"); + let data: Vec = vec![31, 32]; + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let container = root.create_group("container").unwrap(); + let inner = container.create_group("inner").unwrap(); + let ds = inner.new_dataset::().shape([2]).create("ds").unwrap(); + ds.write_raw(&data).unwrap(); + root.link("inner_alias", "/container/inner").unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_group("container").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("inner_alias/ds") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the linked inner group must survive the cross-session delete" + ); + assert!( + file.dataset("container/inner/ds").is_err(), + "the old path must not resolve" + ); + } + + cleanup(&path); +} + +/// Writer-mode paths traverse group hard links the way HDF5 paths do: +/// a group handle opened through an alias creates its objects in the +/// link's target group, attributes land there too, and the results +/// resolve under both the alias and the tree path. +#[test] +fn writer_paths_traverse_group_hard_links() { + let path = unique_tmp("hl_traverse"); + let data: Vec = (0..4).collect(); + + { + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let container = root.create_group("container").unwrap(); + container.create_group("inner").unwrap(); + root.link("inner_alias", "/container/inner").unwrap(); + + // Create and annotate through the alias handle. + let via_alias = root.group("inner_alias").unwrap(); + let ds = via_alias + .new_dataset::() + .shape([4]) + .create("ds") + .unwrap(); + ds.write_raw(&data).unwrap(); + via_alias.set_attr_string("NX_class", "NXdata").unwrap(); + + // A full path through the alias resolves for the writer too. + file.dataset_writer("inner_alias/ds").unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("container/inner/ds") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the object created via the alias must live in the target group" + ); + assert_eq!( + file.dataset("inner_alias/ds") + .unwrap() + .read_raw::() + .unwrap(), + data, + "the alias path must resolve to it as well" + ); + let inner = file.root_group().group("container").unwrap(); + let inner = inner.group("inner").unwrap(); + assert_eq!( + inner.attr_string("NX_class").unwrap(), + "NXdata", + "the attribute set via the alias must land on the target" + ); + } + + cleanup(&path); +} + +/// Deleting through an alias directory resolves the directory but keeps +/// the leaf literal — `H5Ldelete` deletes the named link, not what a +/// leaf alias points at. +#[test] +fn delete_through_an_alias_directory() { + let path = unique_tmp("hl_del_traverse"); + + let file = H5File::create(&path).unwrap(); + let root = file.root_group(); + let container = root.create_group("container").unwrap(); + let inner = container.create_group("inner").unwrap(); + inner.new_dataset::().shape([2]).create("ds").unwrap(); + root.link("inner_alias", "/container/inner").unwrap(); + + // "inner_alias/ds" resolves to the tree name "container/inner/ds". + file.delete_dataset("inner_alias/ds").unwrap(); + assert!( + file.dataset_writer("container/inner/ds").is_err(), + "the delete must reach the dataset behind the alias directory" + ); + + drop(file); + cleanup(&path); +} + /// A target path given with a trailing slash still resolves. #[test] fn hard_link_tolerates_trailing_slash() { diff --git a/tests/reopen_chunk_index.rs b/tests/reopen_chunk_index.rs new file mode 100644 index 0000000..1984882 --- /dev/null +++ b/tests/reopen_chunk_index.rs @@ -0,0 +1,286 @@ +//! Reopened fixed-array and v2-B-tree chunk indexes. +//! +//! `open_append` used to rebuild only the extensible-array index and leave +//! FA/BT2 datasets as re-link placeholders: writes to them were refused, +//! and — worse — deleting one freed just the attributes and object header +//! while every chunk block and the index structures leaked. The index is +//! now read back like the EA one, so reopened FA/BT2 datasets take writes +//! and deletes reclaim their storage. + +use std::path::PathBuf; +use std::sync::atomic::{AtomicU64, Ordering}; + +use rust_hdf5::H5File; + +fn unique_tmp(label: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "rust_hdf5_reopen_index_{}_{}_{}", + label, + std::process::id(), + n + )); + std::fs::create_dir_all(&dir).unwrap(); + dir.join(format!("{label}.h5")) +} + +fn cleanup(path: &PathBuf) { + let _ = std::fs::remove_file(path); + if let Some(dir) = path.parent() { + let _ = std::fs::remove_dir_all(dir); + } +} + +fn make_fa(file: &H5File, vals: &[i32]) { + let ds = file + .new_dataset::() + .shape([4usize, 4]) + .chunk(&[1, 4]) + .max_shape(&[Some(8), Some(4)]) + .create("grid") + .unwrap(); + ds.write_slice(&[0, 0], &[4, 4], vals).unwrap(); +} + +fn make_bt2(file: &H5File, vals: &[i32]) { + let ds = file + .new_dataset::() + .shape([4usize, 4]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .create("tiles") + .unwrap(); + ds.write_slice(&[0, 0], &[4, 4], vals).unwrap(); +} + +/// Deleting a reopened FA dataset must free the chunks it read back off +/// the disk, plus the FA header and data block — the settled-size oracle +/// of `delete_reclamation.rs`, across sessions. +#[test] +fn reopen_session_delete_frees_fixed_array_storage() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("fa_del_{cycles}")); + let vals: Vec = (0..16).collect(); + { + let file = H5File::create(&path).unwrap(); + make_fa(&file, &vals); + file.close().unwrap(); + } + for _ in 0..cycles { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_dataset("grid").unwrap(); + make_fa(&file, &vals); + file.close().unwrap(); + } + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("grid").unwrap().read_raw::().unwrap(), + vals + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(10), size_after(2), "10 reopen cycles against 2"); +} + +/// Deleting a reopened BT2 dataset must free the chunks its tree names, +/// the node blocks, and the header. +#[test] +fn reopen_session_delete_frees_btree_v2_storage() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("bt2_del_{cycles}")); + let vals: Vec = (100..116).collect(); + { + let file = H5File::create(&path).unwrap(); + make_bt2(&file, &vals); + file.close().unwrap(); + } + for _ in 0..cycles { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_dataset("tiles").unwrap(); + make_bt2(&file, &vals); + file.close().unwrap(); + } + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("tiles").unwrap().read_raw::().unwrap(), + vals + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(10), size_after(2), "10 reopen cycles against 2"); +} + +/// A reopened FA dataset takes writes into chunk slots the first session +/// never allocated, and the second session's index flush makes them +/// readable — the flush used to be gated on the EA index alone. +#[test] +fn reopened_fixed_array_dataset_takes_new_chunks() { + let path = unique_tmp("fa_write"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([8usize, 4]) + .chunk(&[1, 4]) + .max_shape(&[Some(8), Some(4)]) + .create("grid") + .unwrap(); + // Rows 0..4 only; rows 4..8 stay unallocated (incremental). + let top: Vec = (0..16).collect(); + ds.write_slice(&[0, 0], &[4, 4], &top).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + let ds = file.dataset_writer("grid").unwrap(); + let bottom: Vec = (16..32).collect(); + ds.write_slice(&[4, 0], &[4, 4], &bottom).unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let all: Vec = (0..32).collect(); + assert_eq!( + file.dataset("grid").unwrap().read_raw::().unwrap(), + all + ); + drop(file); + cleanup(&path); +} + +/// A paged FA data block (more than 1024 chunks) reconstructs on reopen: +/// session 1 initializes only page 0, so page 1's bitmap bit is clear and +/// its on-disk bytes are never decoded; session 2 writes through both +/// pages and the whole array reads back. `open_append` used to leave any +/// paged FA dataset as a re-link placeholder that refused writes. +#[test] +fn reopened_paged_fixed_array_dataset_takes_new_chunks() { + let path = unique_tmp("fa_paged_write"); + let n = 1200usize; + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([n]) + .chunk(&[1]) + .max_shape(&[Some(n)]) + .create("wide") + .unwrap(); + // Elements 0..600 live in page 0 (elements 0..1024); page 1 + // (elements 1024..1200) stays uninitialized on disk. + let front: Vec = (0..600).collect(); + ds.write_slice(&[0], &[600], &front).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + let ds = file.dataset_writer("wide").unwrap(); + let back: Vec = (600..n as i32).collect(); + ds.write_slice(&[600], &[600], &back).unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let all: Vec = (0..n as i32).collect(); + assert_eq!( + file.dataset("wide").unwrap().read_raw::().unwrap(), + all + ); + drop(file); + cleanup(&path); +} + +/// Deleting a reopened *paged* FA dataset must free all its chunks, the +/// header, and the paged data block — the settled-size oracle again. +#[test] +fn reopen_session_delete_frees_paged_fixed_array_storage() { + let make = |file: &H5File, vals: &[i32]| { + let ds = file + .new_dataset::() + .shape([1200usize]) + .chunk(&[1]) + .max_shape(&[Some(1200)]) + .create("wide") + .unwrap(); + ds.write_slice(&[0], &[1200], vals).unwrap(); + }; + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("fa_paged_del_{cycles}")); + let vals: Vec = (0..1200).collect(); + { + let file = H5File::create(&path).unwrap(); + make(&file, &vals); + file.close().unwrap(); + } + for _ in 0..cycles { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + file.delete_dataset("wide").unwrap(); + make(&file, &vals); + file.close().unwrap(); + } + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("wide").unwrap().read_raw::().unwrap(), + vals + ); + drop(read); + let n = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + n + }; + + assert_eq!(size_after(6), size_after(2), "6 reopen cycles against 2"); +} + +/// The filtered-BT2 counterpart: compressed records decode back into the +/// index (address, stored size, filter mask), a reopened write patches +/// and adds tiles, and the whole grid reads back. +#[cfg(feature = "deflate")] +#[test] +fn reopened_filtered_btree_v2_dataset_takes_new_chunks() { + let path = unique_tmp("bt2_write"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4usize, 4]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .deflate(4) + .create("tiles") + .unwrap(); + // Top half only: tiles (0,0) and (0,1). + let top: Vec = (0..8).collect(); + ds.write_slice(&[0, 0], &[2, 4], &top).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + let ds = file.dataset_writer("tiles").unwrap(); + // Bottom half is new tiles; patching (0,0) recompresses an + // existing record to a new size and address. + let bottom: Vec = (8..16).collect(); + ds.write_slice(&[2, 0], &[2, 4], &bottom).unwrap(); + ds.write_slice(&[0, 0], &[1, 1], &[99i32]).unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let mut expect: Vec = (0..16).collect(); + expect[0] = 99; + assert_eq!( + file.dataset("tiles").unwrap().read_raw::().unwrap(), + expect + ); + drop(file); + cleanup(&path); +} diff --git a/tests/set_extent.rs b/tests/set_extent.rs index 1fbf703..a7d9861 100644 --- a/tests/set_extent.rs +++ b/tests/set_extent.rs @@ -2,8 +2,11 @@ //! //! Unlike `extend`, `set_extent` can shrink a chunked dataset's logical //! dimensions — the way to correct an over-extended frame count after a -//! partial multi-frame chunk. Data in chunks beyond the new extent stays in -//! the file but is no longer visible on read. +//! partial multi-frame chunk. A shrink prunes stored chunks the way +//! libhdf5's `H5D__chunk_prune_by_extent` does: chunks entirely beyond the +//! new extent are de-indexed and their storage freed, and chunks the new +//! extent cuts through get their out-of-extent region refilled with the +//! fill value. use std::path::PathBuf; use std::sync::atomic::{AtomicU64, Ordering}; @@ -74,6 +77,282 @@ fn set_extent_shrinks_logical_extent() { cleanup(&path); } +/// An extent change made in a reopen session with no chunk write survives +/// the close: the finalize path must rebuild the dataset header for it, +/// not just when chunks were written. +#[test] +fn set_extent_alone_survives_a_reopen_session() { + let path = unique_tmp("bare_extent"); + + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0usize, 4]) + .chunk(&[1, 4]) + .max_shape(&[None, Some(4)]) + .create("data") + .unwrap(); + for f in 0..2i32 { + let row = [f, f + 1, f + 2, f + 3]; + let raw: Vec = row.iter().flat_map(|v| v.to_le_bytes()).collect(); + ds.write_chunk(f as usize, &raw).unwrap(); + } + ds.extend(&[2, 4]).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::options().no_locking().open_rw(&path).unwrap(); + let ds = file.dataset_writer("data").unwrap(); + ds.set_extent(&[4, 4]).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("data").unwrap(); + assert_eq!(ds.shape(), vec![4, 4]); + } + + cleanup(&path); +} + +/// Shrinking an extensible-array dataset prunes the stranded chunks, so +/// growing the extent back reads fill values — not the stale data. Twelve +/// chunks with `idx_blk_elmts = 4` puts the pruned entries in both the EA +/// index block and on-disk data blocks. +#[test] +fn ea_shrink_then_regrow_reads_fill_not_stale() { + let path = unique_tmp("ea_regrow"); + + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([24usize, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(4)]) + .create("data") + .unwrap(); + let vals: Vec = (0..24 * 4).collect(); + ds.write_slice(&[0, 0], &[24, 4], &vals).unwrap(); + // Chunk 1 (rows 2..4) straddles the new extent; chunks 2..12 are + // entirely beyond it. + ds.set_extent(&[3, 4]).unwrap(); + ds.set_extent(&[24, 4]).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("data").unwrap(); + assert_eq!(ds.shape(), vec![24, 4]); + let v = ds.read_raw::().unwrap(); + for r in 0..24usize { + for c in 0..4usize { + let expect = if r < 3 { (r * 4 + c) as i32 } else { 0 }; + assert_eq!(v[r * 4 + c], expect, "row {r} col {c}"); + } + } + } + + cleanup(&path); +} + +/// The same prune-and-refill through the filtered extensible-array path: +/// stored chunk sizes vary, so the freed lengths come from the index +/// entries and the refilled straddler is re-compressed. +#[cfg(feature = "deflate")] +#[test] +fn ea_filtered_shrink_then_regrow_reads_fill() { + let path = unique_tmp("ea_filt_regrow"); + + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([24usize, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(4)]) + .deflate(4) + .create("data") + .unwrap(); + let vals: Vec = (0..24 * 4).map(|i| i / 4).collect(); + ds.write_slice(&[0, 0], &[24, 4], &vals).unwrap(); + ds.set_extent(&[3, 4]).unwrap(); + ds.set_extent(&[24, 4]).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("data").unwrap(); + let v = ds.read_raw::().unwrap(); + for r in 0..24usize { + for c in 0..4usize { + let expect = if r < 3 { r as i32 } else { 0 }; + assert_eq!(v[r * 4 + c], expect, "row {r} col {c}"); + } + } + } + + cleanup(&path); +} + +/// A shrink frees the pruned chunks' storage for reuse: writing a second +/// dataset's chunks after the shrink consumes exactly the freed blocks, so +/// the file ends up as large as one that never wrote the pruned chunks. +/// (Both datasets' index blocks are created before the shrink, so the +/// post-shrink chunk writes are the only allocations left — an exact-size +/// reuse with no fragmentation.) +#[test] +fn fa_shrink_frees_chunk_storage_for_reuse() { + let path1 = unique_tmp("fa_reuse_shrunk"); + let path2 = unique_tmp("fa_reuse_reference"); + + // Shrunk file: A's chunks 1..4 (rows 2..8) are freed, then B's three + // chunks reuse those blocks. + { + let file = H5File::create(&path1).unwrap(); + let a = file + .new_dataset::() + .shape([8usize, 4]) + .chunk(&[2, 4]) + .create("a") + .unwrap(); + let vals: Vec = (0..8 * 4).collect(); + a.write_slice(&[0, 0], &[8, 4], &vals).unwrap(); + let b = file + .new_dataset::() + .shape([6usize, 4]) + .chunk(&[2, 4]) + .create("b") + .unwrap(); + a.set_extent(&[2, 4]).unwrap(); + let vals: Vec = (0..6 * 4).collect(); + b.write_slice(&[0, 0], &[6, 4], &vals).unwrap(); + file.close().unwrap(); + } + + // Reference file: identical structure, but A never writes the chunks + // the other file pruned, and B's chunks are fresh allocations. + { + let file = H5File::create(&path2).unwrap(); + let a = file + .new_dataset::() + .shape([8usize, 4]) + .chunk(&[2, 4]) + .create("a") + .unwrap(); + let vals: Vec = (0..2 * 4).collect(); + a.write_slice(&[0, 0], &[2, 4], &vals).unwrap(); + let b = file + .new_dataset::() + .shape([6usize, 4]) + .chunk(&[2, 4]) + .create("b") + .unwrap(); + let vals: Vec = (0..6 * 4).collect(); + b.write_slice(&[0, 0], &[6, 4], &vals).unwrap(); + file.close().unwrap(); + } + + let s1 = std::fs::metadata(&path1).unwrap().len(); + let s2 = std::fs::metadata(&path2).unwrap().len(); + assert_eq!( + s1, s2, + "shrunk-then-reused file should be as large as one that never wrote \ + the pruned chunks" + ); + + cleanup(&path1); + cleanup(&path2); +} + +/// Fixed-array prune-and-refill read back: pruned region reads fill after a +/// regrow, retained region is intact, straddlers in both dimensions. +#[test] +fn fa_shrink_then_regrow_reads_fill() { + let path = unique_tmp("fa_regrow"); + + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([6usize, 6]) + .chunk(&[2, 3]) + .create("data") + .unwrap(); + let vals: Vec = (0..36).collect(); + ds.write_slice(&[0, 0], &[6, 6], &vals).unwrap(); + // [3, 4] cuts through chunks in both dimensions. + ds.set_extent(&[3, 4]).unwrap(); + ds.set_extent(&[6, 6]).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("data").unwrap(); + let v = ds.read_raw::().unwrap(); + for r in 0..6usize { + for c in 0..6usize { + let expect = if r < 3 && c < 4 { + (r * 6 + c) as i32 + } else { + 0 + }; + assert_eq!(v[r * 6 + c], expect, "row {r} col {c}"); + } + } + } + + cleanup(&path); +} + +/// V2-B-tree prune-and-refill read back: records beyond the extent leave +/// the tree (the node pool releases the surplus at flush), straddlers are +/// refilled in both dimensions. +#[test] +fn bt2_shrink_then_regrow_reads_fill() { + let path = unique_tmp("bt2_regrow"); + + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4usize, 6]) + .chunk(&[2, 3]) + .max_shape(&[None, None]) + .create("data") + .unwrap(); + let vals: Vec = (0..24).collect(); + ds.write_slice(&[0, 0], &[4, 6], &vals).unwrap(); + ds.set_extent(&[3, 4]).unwrap(); + ds.set_extent(&[4, 6]).unwrap(); + file.close().unwrap(); + } + + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("data").unwrap(); + let v = ds.read_raw::().unwrap(); + for r in 0..4usize { + for c in 0..6usize { + let expect = if r < 3 && c < 4 { + (r * 6 + c) as i32 + } else { + 0 + }; + assert_eq!(v[r * 6 + c], expect, "row {r} col {c}"); + } + } + } + + cleanup(&path); +} + /// `set_extent` rejects an extent above the dataset's maximum dimensions. #[test] fn set_extent_rejects_exceeding_max() { @@ -95,3 +374,120 @@ fn set_extent_rejects_exceeding_max() { drop(file); cleanup(&path); } + +// ---- vlen datasets: a shrink must release the stranded heap objects ------ + +/// Chunk-aligned shrink of a vlen dataset: every pruned chunk's strings are +/// global-heap objects, and freeing the chunk block without releasing them +/// strands one `H5HG_MINALLOC` (4096-byte) collection per append/shrink +/// cycle. With the release the cycle reuses the same blocks and the file +/// size settles. +#[test] +fn vlen_shrink_releases_the_pruned_chunks_heap_objects() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("vlen_prune_release_{cycles}")); + let file = H5File::create(&path).unwrap(); + file.create_appendable_vlen_dataset("notes", 2, None) + .unwrap(); + file.append_vlen_strings("notes", &["keep0", "keep1"]) + .unwrap(); + for i in 0..cycles { + // One full chunk per cycle, so nothing stays in the append + // buffer (a buffered append blocks `set_extent`). + file.append_vlen_strings("notes", &[&format!("x{i}"), &format!("y{i}")]) + .unwrap(); + file.dataset_writer("notes") + .unwrap() + .set_extent(&[2]) + .unwrap(); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["keep0", "keep1"] + ); + drop(read); + cleanup(&path); + n + }; + + let settled = size_after(3); + assert_eq!(size_after(20), settled, "20 cycles against 3"); + assert_eq!(size_after(50), settled, "50 cycles against 3"); +} + +/// A shrink that cuts through a chunk refills its out-of-extent tail with +/// the fill value; the strings the refill overwrites must be released too, +/// or every cycle strands the replaced element's collection. +#[test] +fn vlen_shrink_releases_the_refilled_straddlers_heap_objects() { + let size_after = |cycles: usize| { + let path = unique_tmp(&format!("vlen_straddle_release_{cycles}")); + let file = H5File::create(&path).unwrap(); + file.create_appendable_vlen_dataset("notes", 2, None) + .unwrap(); + file.append_vlen_strings("notes", &["keep0", "stub"]) + .unwrap(); + let ds = file.dataset_writer("notes").unwrap(); + ds.set_extent(&[1]).unwrap(); + for i in 0..cycles { + // Regrow, write row 1 directly into the chunk (a slice write + // flushes, it never buffers), shrink it back out: the shrink's + // straddler refill is the only thing that can release `x{i}`. + ds.set_extent(&[2]).unwrap(); + ds.write_vlen_strings_slice(1, &[&format!("x{i}")]).unwrap(); + ds.set_extent(&[1]).unwrap(); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["keep0"] + ); + drop(read); + cleanup(&path); + n + }; + + let settled = size_after(3); + assert_eq!(size_after(20), settled, "20 cycles against 3"); + assert_eq!(size_after(50), settled, "50 cycles against 3"); +} + +/// Survivor strings stay intact through a shrink and a regrown row reads +/// as the fill value — the nil reference, i.e. the empty string — for both +/// an unfiltered and a deflate-compressed vlen dataset (the compressed one +/// must decompress the pruned chunk before parsing its references). +#[test] +fn vlen_shrink_then_regrow_keeps_survivors_and_reads_empty() { + let mut variants: Vec<(&str, Option)> = vec![("plain", None)]; + if cfg!(feature = "deflate") { + variants.push(("deflate", Some(rust_hdf5::FilterPipeline::deflate(4)))); + } + for (tag, pipeline) in variants { + let path = unique_tmp(&format!("vlen_regrow_{tag}")); + { + let file = H5File::create(&path).unwrap(); + file.create_appendable_vlen_dataset("notes", 2, pipeline) + .unwrap(); + file.append_vlen_strings("notes", &["s0", "s1", "s2", "s3", "s4", "s5"]) + .unwrap(); + let ds = file.dataset_writer("notes").unwrap(); + // Chunk 1 straddles (s3 refilled), chunk 2 is pruned (s4, s5). + ds.set_extent(&[3]).unwrap(); + ds.set_extent(&[6]).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["s0", "s1", "s2", "", "", ""], + "{tag}" + ); + drop(file); + cleanup(&path); + } +} diff --git a/tests/swmr_full_api.rs b/tests/swmr_full_api.rs index ba164e6..e04a838 100644 --- a/tests/swmr_full_api.rs +++ b/tests/swmr_full_api.rs @@ -453,3 +453,82 @@ fn swmr_chunk_rewrite_does_not_grow_the_file() { cleanup(&once_path); cleanup(&many_path); } + +/// Boundary: the typed reads check `T` against the stored element width. +/// An f64 dataset read as `i32` used to pass the divisibility check and +/// return twice as many garbage values. +#[test] +fn typed_reads_reject_a_mismatched_element_width() { + let path = unique_tmp("typed_width"); + { + let mut w = SwmrFileWriter::create_with_locking(&path, NO_LOCK).unwrap(); + w.write_dataset::("d", &[2], &[1.5, -2.25]).unwrap(); + w.close().unwrap(); + } + let mut r = SwmrFileReader::open_with_locking(&path, NO_LOCK).unwrap(); + assert_eq!(r.read_dataset::("d").unwrap(), vec![1.5, -2.25]); + + let err = r.read_dataset::("d").unwrap_err(); + assert!( + err.to_string().contains("element size"), + "unexpected error: {err}" + ); + let err = r.read_slice::("d", &[0], &[1]).unwrap_err(); + assert!( + err.to_string().contains("element size"), + "unexpected error: {err}" + ); + assert_eq!(r.read_slice::("d", &[1], &[1]).unwrap(), vec![-2.25]); + cleanup(&path); +} + +/// Every attribute mutation after `start_swmr` is refused, libhdf5's rule +/// for SWMR writes. Object headers are frozen once streaming starts, so a +/// change only reached the file when its header happened to be rebuilt at +/// close (group attrs always, dataset attrs only alongside chunk writes) +/// and was silently dropped otherwise — and replacing a vlen value +/// stranded its old 4096-byte collection forever, since a streaming +/// reader may still hold the references. +#[test] +fn attribute_changes_during_swmr_are_refused() { + let path = unique_tmp("swmr_attr_freeze"); + { + let mut w = SwmrFileWriter::create_with_locking(&path, NO_LOCK).unwrap(); + w.create_group("/", "entry").unwrap(); + w.set_group_attr_string("/entry", "NX_class", "NXentry") + .unwrap(); + let ds = w.create_streaming_dataset::("frames", &[4]).unwrap(); + w.set_dataset_attr_string(ds, "units", "mm").unwrap(); + w.set_dataset_attr_numeric(ds, "scale", &2i32).unwrap(); + w.start_swmr().unwrap(); + + let err = w + .set_group_attr_string("/entry", "NX_class", "NXdata") + .expect_err("vlen replace under SWMR must be refused"); + assert!(format!("{err}").contains("SWMR"), "got: {err}"); + w.set_dataset_attr_string(ds, "units", "cm") + .expect_err("vlen replace under SWMR must be refused"); + w.set_dataset_attr_string(ds, "long_name", "detector x") + .expect_err("a new vlen attribute under SWMR must be refused"); + w.set_dataset_attr_numeric(ds, "scale", &3i32) + .expect_err("a numeric replace under SWMR must be refused"); + w.close().unwrap(); + } + + // No stranded collection: the refused post-start calls must not have + // written a heap block. The two pre-start vlen attrs pack into one + // shared collection, so exactly one "GCOL" may appear. + let bytes = std::fs::read(&path).unwrap(); + let gcols = bytes.windows(4).filter(|w| *w == b"GCOL").count(); + assert_eq!(gcols, 1, "only the shared pre-start collection may exist"); + + // The pre-start values are what the file holds. + let file = rust_hdf5::H5File::open(&path).unwrap(); + let entry = file.root_group().group("entry").unwrap(); + assert_eq!(entry.attr_string("NX_class").unwrap(), "NXentry"); + let ds = file.dataset("frames").unwrap(); + assert_eq!(ds.attr("units").unwrap().read_string().unwrap(), "mm"); + assert_eq!(ds.attr("scale").unwrap().read_numeric::().unwrap(), 2); + drop(file); + cleanup(&path); +} diff --git a/tests/vlen_heap_hardening.rs b/tests/vlen_heap_hardening.rs new file mode 100644 index 0000000..f19aa7a --- /dev/null +++ b/tests/vlen_heap_hardening.rs @@ -0,0 +1,237 @@ +//! Global-heap resolution hardening for variable-length data. +//! +//! A reference whose collection cannot be resolved must be a hard error, +//! the way libhdf5's `H5HG__cache_heap_deserialize` fails on a bad +//! signature or an undersized collection — not a silent run of empty +//! strings. And a collection larger than 64 MiB is not an error at all: +//! libhdf5 has no upper cap, and this crate's writers put a whole write +//! call's strings into one collection, so a cap silently blanks every +//! string of a large batch. + +use std::path::PathBuf; +use std::sync::atomic::{AtomicU64, Ordering}; + +use rust_hdf5::H5File; + +fn unique_tmp(label: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "rust_hdf5_vlen_heap_{}_{}_{}", + label, + std::process::id(), + n + )); + std::fs::create_dir_all(&dir).unwrap(); + dir.join(format!("{label}.h5")) +} + +fn cleanup(path: &PathBuf) { + let _ = std::fs::remove_file(path); + if let Some(dir) = path.parent() { + let _ = std::fs::remove_dir_all(dir); + } +} + +/// Find the file's first global-heap collection and hand its bytes to +/// `mutate`. The tests below write exactly one collection per file, so +/// "first" is "the" collection. +fn patch_gcol(path: &PathBuf, mutate: impl FnOnce(&mut [u8])) { + let mut bytes = std::fs::read(path).unwrap(); + let off = bytes + .windows(4) + .position(|w| w == b"GCOL") + .expect("no GCOL collection in file"); + mutate(&mut bytes[off..]); + std::fs::write(path, &bytes).unwrap(); +} + +/// One write call's strings all go into a single collection, so 66 x 1 MiB +/// puts the collection past the reader's former 64 MiB cap — which turned +/// every one of them into an empty string on read-back. +#[test] +fn vlen_collection_above_64_mib_roundtrips() { + let path = unique_tmp("big_collection"); + let strings: Vec = (0..66) + .map(|i| { + let c = char::from(b'a' + (i % 26) as u8); + std::iter::repeat_n(c, 1 << 20).collect() + }) + .collect(); + let refs: Vec<&str> = strings.iter().map(|s| s.as_str()).collect(); + + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("notes", &refs).unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let read = file.dataset("notes").unwrap().read_vlen_strings().unwrap(); + assert_eq!(read, strings); + drop(file); + cleanup(&path); +} + +/// A collection whose signature is gone is corruption, and reading a +/// string through it must fail loudly instead of yielding "". +#[test] +fn corrupt_gcol_signature_is_a_dataset_read_error() { + let path = unique_tmp("bad_signature"); + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("notes", &["alpha", "beta"]) + .unwrap(); + file.close().unwrap(); + } + patch_gcol(&path, |g| g[..4].copy_from_slice(b"XXXX")); + + let file = H5File::open(&path).unwrap(); + let err = file + .dataset("notes") + .unwrap() + .read_vlen_strings() + .expect_err("corrupt collection must not read as empty strings"); + assert!(format!("{err}").contains("signature"), "got: {err}"); + drop(file); + cleanup(&path); +} + +/// A declared collection size below `H5HG_MINSIZE` (4096) is rejected the +/// way libhdf5 rejects it. +#[test] +fn undersized_gcol_declared_size_is_a_read_error() { + let path = unique_tmp("undersized"); + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("notes", &["alpha", "beta"]) + .unwrap(); + file.close().unwrap(); + } + // Collection size is the 8-byte field after signature+version+reserved. + patch_gcol(&path, |g| g[8..16].copy_from_slice(&100u64.to_le_bytes())); + + let file = H5File::open(&path).unwrap(); + let err = file + .dataset("notes") + .unwrap() + .read_vlen_strings() + .expect_err("undersized collection must not read as empty strings"); + assert!(format!("{err}").contains("4096"), "got: {err}"); + drop(file); + cleanup(&path); +} + +/// The attribute path resolves through the same collection loader, so a +/// corrupt collection fails an attribute string read too. +#[test] +fn corrupt_gcol_signature_is_an_attr_read_error() { + let path = unique_tmp("bad_signature_attr"); + { + let file = H5File::create(&path).unwrap(); + file.set_attr_string("conventions", "NeXus").unwrap(); + file.close().unwrap(); + } + patch_gcol(&path, |g| g[..4].copy_from_slice(b"XXXX")); + + let file = H5File::open(&path).unwrap(); + let err = file + .attr_string("conventions") + .expect_err("corrupt collection must not read as an empty attribute"); + assert!(format!("{err}").contains("signature"), "got: {err}"); + drop(file); + cleanup(&path); +} + +/// An append the dataset refuses (here: a contiguous vlen dataset — only +/// chunked ones are appendable) must be rejected before the heap write, +/// or every failed call orphans a 4096-byte collection. +#[test] +fn rejected_append_does_not_orphan_a_collection() { + let size_after = |attempts: usize| { + let path = unique_tmp(&format!("rejected_append_{attempts}")); + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("notes", &["a", "b"]).unwrap(); + for _ in 0..attempts { + file.append_vlen_strings("notes", &["x"]) + .expect_err("append on a contiguous dataset must be rejected"); + } + file.close().unwrap(); + let n = std::fs::metadata(&path).unwrap().len(); + let read = H5File::open(&path).unwrap(); + assert_eq!( + read.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["a", "b"] + ); + drop(read); + cleanup(&path); + n + }; + + assert_eq!( + size_after(20), + size_after(1), + "20 rejected appends against 1" + ); +} + +/// A zero-element vlen dataset or string-array attribute references no +/// heap object, so no collection belongs in the file — an empty +/// collection is still a 4096-byte `H5HG_MINALLOC` block nothing points +/// to. The file must contain no `GCOL` block at all, and the empty value +/// must read back. +#[test] +fn empty_vlen_creators_write_no_collection() { + type WriteEmpty = fn(&H5File); + let cases: &[(&str, WriteEmpty)] = &[ + ("strings", |f| { + f.write_vlen_strings("v", &[]).unwrap(); + }), + ("bytes", |f| { + let no_items: &[&[u8]] = &[]; + f.write_vlen_bytes("v", no_items).unwrap(); + }), + ("compressed", |f| { + f.write_vlen_strings_compressed("v", &[], 4, rust_hdf5::FilterPipeline::deflate(4)) + .unwrap(); + }), + ("attr_array", |f| { + f.set_attr_string_array("v", &[]).unwrap(); + }), + ]; + + for (tag, write) in cases { + let path = unique_tmp(&format!("empty_{tag}")); + let file = H5File::create(&path).unwrap(); + write(&file); + file.close().unwrap(); + + let bytes = std::fs::read(&path).unwrap(); + assert!( + !bytes.windows(4).any(|w| w == b"GCOL"), + "{tag}: empty input must not write a heap collection" + ); + + let read = H5File::open(&path).unwrap(); + match *tag { + "strings" | "compressed" => { + assert_eq!( + read.dataset("v").unwrap().read_vlen_strings().unwrap(), + Vec::::new(), + "{tag}" + ); + } + "bytes" => { + assert_eq!( + read.dataset("v").unwrap().read_vlen_bytes().unwrap(), + Vec::>::new() + ); + } + _ => { + assert!(read.attr_names().unwrap().contains(&"v".to_string())); + } + } + drop(read); + cleanup(&path); + } +} diff --git a/tests/vlen_heap_packing.rs b/tests/vlen_heap_packing.rs new file mode 100644 index 0000000..4943ec0 --- /dev/null +++ b/tests/vlen_heap_packing.rs @@ -0,0 +1,280 @@ +//! Writer-side global-heap packing (libhdf5's CWFS list, `H5HG_insert`). +//! +//! Every vlen write call used to allocate its own collection, so a file of +//! small strings paid the 4096-byte `H5HG_MINALLOC` minimum per call — and +//! a batch of more than 65535 strings was a hard error, because one call +//! meant one collection and a collection's object index is 16-bit. Now +//! `insert_vlen_objects` packs into collections with free space and spills +//! into a fresh collection at the index cap. +//! +//! The oracle for sharing is the number of `GCOL` signatures in the file: +//! these tests never free a whole collection block, so no stale signature +//! can inflate the count, and none of the written strings contain the +//! bytes `GCOL`. + +use std::path::PathBuf; +use std::sync::atomic::{AtomicU64, Ordering}; + +use rust_hdf5::H5File; + +fn unique_tmp(label: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "rust_hdf5_vlen_packing_{}_{}_{}", + label, + std::process::id(), + n + )); + std::fs::create_dir_all(&dir).unwrap(); + dir.join(format!("{label}.h5")) +} + +fn cleanup(path: &PathBuf) { + let _ = std::fs::remove_file(path); + if let Some(dir) = path.parent() { + let _ = std::fs::remove_dir_all(dir); + } +} + +fn gcol_count(path: &PathBuf) -> usize { + let bytes = std::fs::read(path).unwrap(); + bytes.windows(4).filter(|w| *w == b"GCOL").count() +} + +/// Six string attributes used to mean six 4096-byte collections; they all +/// fit one. +#[test] +fn small_attributes_share_one_collection() { + let path = unique_tmp("attrs"); + { + let file = H5File::create(&path).unwrap(); + let g = file.root_group().create_group("entry").unwrap(); + for (name, value) in [ + ("NX_class", "NXentry"), + ("definition", "NXtomo"), + ("title", "sample scan"), + ("operator", "kim"), + ("start_time", "2026-08-13T09:00:00"), + ("end_time", "2026-08-13T09:20:00"), + ] { + g.set_attr_string(name, value).unwrap(); + } + file.close().unwrap(); + } + + assert_eq!(gcol_count(&path), 1, "six small attrs, one collection"); + let file = H5File::open(&path).unwrap(); + let g = file.root_group().group("entry").unwrap(); + assert_eq!(g.attr_string("NX_class").unwrap(), "NXentry"); + assert_eq!(g.attr_string("end_time").unwrap(), "2026-08-13T09:20:00"); + drop(file); + cleanup(&path); +} + +/// A vlen attribute too big for the listed collection's remaining free +/// space extends the collection in place — libhdf5's CWFS second pass +/// (`H5F_cwfs_find_free_heap`: `H5MF_try_extend` + `H5HG_extend`) — +/// instead of opening a second one. +#[test] +fn an_oversized_attr_extends_the_collection_instead_of_opening_a_second() { + let path = unique_tmp("extend"); + let big = "x".repeat(5000); + { + let file = H5File::create(&path).unwrap(); + let g = file.root_group().create_group("entry").unwrap(); + g.set_attr_string("small", "hello").unwrap(); + g.set_attr_string("big", &big).unwrap(); + file.close().unwrap(); + } + + assert_eq!( + gcol_count(&path), + 1, + "the big attr opened a second collection" + ); + let file = H5File::open(&path).unwrap(); + let g = file.root_group().group("entry").unwrap(); + assert_eq!(g.attr_string("small").unwrap(), "hello"); + assert_eq!(g.attr_string("big").unwrap(), big); + drop(file); + cleanup(&path); +} + +/// Separate write calls — two contiguous datasets and two append batches — +/// share one collection while it has room. +#[test] +fn small_vlen_datasets_share_one_collection() { + let path = unique_tmp("datasets"); + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("notes", &["alpha", "beta", "gamma"]) + .unwrap(); + file.write_vlen_strings("tags", &["red", "green"]).unwrap(); + file.create_appendable_vlen_dataset("log", 2, None).unwrap(); + file.append_vlen_strings("log", &["l0", "l1"]).unwrap(); + file.append_vlen_strings("log", &["l2", "l3"]).unwrap(); + file.close().unwrap(); + } + + assert_eq!(gcol_count(&path), 1, "four write calls, one collection"); + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("notes").unwrap().read_vlen_strings().unwrap(), + vec!["alpha", "beta", "gamma"] + ); + assert_eq!( + file.dataset("tags").unwrap().read_vlen_strings().unwrap(), + vec!["red", "green"] + ); + assert_eq!( + file.dataset("log").unwrap().read_vlen_strings().unwrap(), + vec!["l0", "l1", "l2", "l3"] + ); + drop(file); + cleanup(&path); +} + +/// The exact fit rule: an object packs only when the collection's free +/// space holds it *plus* a residual free-space marker header (16 bytes +/// here). One byte of string past that boundary must start a fresh +/// collection, not corrupt the shared one. +/// +/// Geometry (8-byte lengths): block 4096, header 16, marker header 16. A +/// 4000-byte string leaves free = 4096 - 16 - (16 + 4000) = 64. A 32-byte +/// string needs 16 + 32 = 48 ≤ 64 - 16 → fits; 33 bytes needs 16 + 40 = +/// 56 > 64 - 16 → fresh block. +#[test] +fn packing_respects_the_free_space_marker_boundary() { + let filler = "x".repeat(4000); + + let fits = unique_tmp("fit"); + { + let file = H5File::create(&fits).unwrap(); + file.write_vlen_strings("big", &[&filler]).unwrap(); + file.write_vlen_strings("small", &["y".repeat(32).as_str()]) + .unwrap(); + file.close().unwrap(); + } + assert_eq!(gcol_count(&fits), 1, "32-byte string fits the 64-byte gap"); + + let overflows = unique_tmp("overflow"); + { + let file = H5File::create(&overflows).unwrap(); + file.write_vlen_strings("big", &[&filler]).unwrap(); + file.write_vlen_strings("small", &["y".repeat(33).as_str()]) + .unwrap(); + file.close().unwrap(); + } + assert_eq!(gcol_count(&overflows), 2, "33-byte string must not fit"); + + for path in [&fits, &overflows] { + let file = H5File::open(path).unwrap(); + assert_eq!( + file.dataset("big").unwrap().read_vlen_strings().unwrap(), + vec![filler.clone()] + ); + let small = &file.dataset("small").unwrap().read_vlen_strings().unwrap()[0]; + assert!(small.chars().all(|c| c == 'y'), "small got {small:?}"); + drop(file); + cleanup(path); + } +} + +/// A batch past the 65535-object index cap spills into a second +/// collection instead of failing — the reporter's "collection is full" +/// error. Every element still reads back through its own reference. +#[test] +fn spilled_batch_past_the_index_cap_roundtrips() { + let path = unique_tmp("spill"); + let n = 65537usize; + let strings = vec!["x"; n]; + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("bulk", &strings).unwrap(); + file.close().unwrap(); + } + + assert_eq!(gcol_count(&path), 2, "65537 objects need two collections"); + let file = H5File::open(&path).unwrap(); + let read = file.dataset("bulk").unwrap().read_vlen_strings().unwrap(); + assert_eq!(read.len(), n); + assert!(read.iter().all(|s| s == "x")); + drop(file); + cleanup(&path); +} + +/// Replacing elements releases the old objects into the collection's free +/// space and the replacements pack back into it: a replace loop settles +/// at one collection and one file size. +#[test] +fn replacing_elements_reuses_the_recovered_heap_space() { + let size_after = |rounds: usize| { + let path = unique_tmp(&format!("replace_{rounds}")); + let file = H5File::create(&path).unwrap(); + let ds = file + .write_vlen_strings("notes", &["one", "two", "three", "four"]) + .unwrap(); + for i in 0..rounds { + ds.write_vlen_strings_slice(0, &[format!("round_{i}").as_str()]) + .unwrap(); + } + file.close().unwrap(); + + assert_eq!(gcol_count(&path), 1, "{rounds} rounds, one collection"); + let read = H5File::open(&path).unwrap(); + let got = read.dataset("notes").unwrap().read_vlen_strings().unwrap(); + assert_eq!( + got, + vec![ + format!("round_{}", rounds - 1), + "two".into(), + "three".into(), + "four".into() + ] + ); + drop(read); + let size = std::fs::metadata(&path).unwrap().len(); + cleanup(&path); + size + }; + + assert_eq!(size_after(20), size_after(2), "20 replace rounds against 2"); +} + +/// Deleting one dataset that shares a collection rewrites the block in +/// place — the survivor keeps its objects — and the recovered space takes +/// the next write. +#[test] +fn deleting_one_sharer_keeps_and_reuses_the_collection() { + let path = unique_tmp("sharer"); + { + let file = H5File::create(&path).unwrap(); + file.write_vlen_strings("doomed", &["d0", "d1", "d2"]) + .unwrap(); + file.write_vlen_strings("keep", &["k0", "k1", "k2"]) + .unwrap(); + file.delete_dataset("doomed").unwrap(); + file.write_vlen_strings("after", &["a0", "a1", "a2"]) + .unwrap(); + file.close().unwrap(); + } + + assert_eq!( + gcol_count(&path), + 1, + "delete rewrote in place, no new block" + ); + let file = H5File::open(&path).unwrap(); + assert_eq!( + file.dataset("keep").unwrap().read_vlen_strings().unwrap(), + vec!["k0", "k1", "k2"] + ); + assert_eq!( + file.dataset("after").unwrap().read_vlen_strings().unwrap(), + vec!["a0", "a1", "a2"] + ); + assert!(file.dataset("doomed").is_err()); + drop(file); + cleanup(&path); +}