Skip to content

Make multi-DDict selection deterministic and clarify dictionary lifetimes - #4929

Open
terrelln wants to merge 6 commits into
meta:devfrom
terrelln:2026-11-11-dict-set-v2
Open

terrelln wants to merge 6 commits into
meta:devfrom
terrelln:2026-11-11-dict-set-v2

Conversation

@terrelln

Copy link
Copy Markdown
Contributor

Summary

This PR fixes dictionary lifetime and selection behavior in the experimental ZSTD_d_refMultipleDDicts API. A populated DDict set now acts as an explicit mapping from frame dictionary IDs to borrowed DDicts, with no implicit default or remembered selection. The separate hash-table lookup and growth fixes are part of PR #4927.

The four commits are:

  1. 8a1bf5f — Clear remaining multi-DDict references.
    Centralizes set cleanup and releases the set when loading an empty dictionary or disabling multi-DDict mode. This makes the documented “return to no-dictionary mode” behavior reliable and lets callers immediately free every formerly registered DDict.

  2. 201aae6 — Make local dictionaries and DDict sets mutually exclusive.
    Loading a dictionary or referencing a prefix discards the set; registering a DDict discards any local dictionary or prefix. This gives dictionary replacement an unambiguous lifetime and prevents two competing sources of dictionary state from coexisting.

  3. 845b46b — Require exact matches for nonzero frame IDs.
    Selects dictionaries only after a complete, valid ordinary-frame header is available, before installing decoding state. Missing nonzero IDs return ZSTD_error_dictionary_wrong instead of falling back to the previous dictionary. ID-less frames use a registered ID-0 DDict when present, or no dictionary otherwise.

  4. 82e0df9 — Resolve set entries independently for each frame.
    Keeps the single-dictionary state empty while a set exists and resolves each frame into a local DDict pointer. Explicit-dictionary APIs remain independent of the set, tracing no longer needs a retained DDict pointer, and implicit set-based legacy decoding is rejected.

    The default DDict was an extremely confusing concept, as it ended up being either the last DDict loaded, or the last DDict used regardless of load order. Also explicitly disables legacy frame decoding when there is a non-empty DDict set. The prior behavior was 100% not correct, so this isn't a big loss.

Design rationale

  • Nonzero IDs require exact matches. Falling back to the last registered or selected dictionary makes decoding depend on unrelated earlier operations. ID-less frames instead use a registered ID-0 DDict when present, or no dictionary otherwise. This allows ordinary dictionary-free frames without requiring an empty DDict registration, while preserving explicit support for raw-content dictionaries.
  • The parameter remains sticky. Loading a dictionary or prefix replaces the active set without disabling ZSTD_d_refMultipleDDicts. The parameter controls subsequent refDDict() registrations; changing dictionary storage does not silently change configuration. Existing single-dictionary and single-use-prefix behavior is preserved.
  • Explicit arguments take precedence. decompress_usingDict(), decompress_usingDDict(), and the bufferless APIs must use the dictionary supplied by their caller, even if the context also contains a set. Legacy decoding remains available through these explicit or single-dictionary paths rather than inheriting an arbitrary set member.
  • Header-parser optimization is left separate. The one-shot set path currently parses the header before dictionary initialization, then uses the established frame decoder. This keeps initialization ordering and existing error handling clear; eliminating the redundant parse can be a focused follow-up.

Testing

Adds regression coverage for immediate dictionary release, replacement in both directions, missing nonzero IDs, registered and absent ID-0 entries, dictionary-free frames mixed with dictionary-backed frames, concatenated and magicless input, fragmented streaming and its one-shot shortcut, explicit-dictionary APIs, prefix compatibility, and the legacy-format boundary. An ID-less frame that requires dictionary bytes also verifies that the fallback does not reuse a previous dictionary.

Passed:

make -C tests -j8 fuzzer legacy
tests/fuzzer -i0 -s0
tests/legacy

Terminate multi-DDict lookup at NULL entries, not dictionary ID zero. Cover missing and registered IDs that collide with a raw-content dictionary, using distinct dictionary contents.
Compare scaled occupancy directly so the table grows at 75% instead of 25%. Count allocations and verify buffered-streaming lookup after rehashing, independently of the one-shot decoder changes.
Drop the borrowed-reference table on empty dictionary loads and when disabling multi-DDict mode. Disabling a populated set also clears its selected dictionary, so callers can release every DDict. Reuse cleanup for reset and refDDict(NULL), document the lifetime rule, and test immediate release.
Loading any dictionary or prefix discards the borrowed DDict set without changing the opt-in parameter. refDDict already clears the local dictionary or prefix before registering a borrowed DDict. Document this invariant and test replacement in both directions, including immediate release of borrowed storage.
Select dictionaries only after a complete ordinary frame header is available, before installing dictionary-backed decoding state. Return dictionary_wrong instead of falling back to the current dictionary when the set has no exact nonzero ID match. Frames with ID 0 use a registered ID-0 DDict if present, or no dictionary otherwise. Document the compatibility change and test missing IDs, dictionary-free concatenated frames, raw-content ID-0 dictionaries, and both streaming paths.
@meta-cla meta-cla Bot added the CLA Signed label Nov 11, 2026
Keep the single-DDict state empty while a DDict set exists. Resolve each ordinary frame into a local DDict pointer before installing decoder state, using no dictionary when ID 0 has no registered entry, and leave explicit-dictionary APIs independent of the set. Preserve trace metadata without retaining a borrowed DDict. Reject implicit set-based legacy decoding, retain explicit legacy decoding, and cover immediate release after disabling a set, raw ID 0, dictionary-free frames, and unchanged local-prefix behavior.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant