Skip to content

Fix races during IDS metadata construction - #145

Open
chupakobra6 wants to merge 1 commit into
iterorganization:developfrom
chupakobra6:codex/fix-metadata-construction-race
Open

Fix races during IDS metadata construction#145
chupakobra6 wants to merge 1 commit into
iterorganization:developfrom
chupakobra6:codex/fix-metadata-construction-race

Conversation

@chupakobra6

@chupakobra6 chupakobra6 commented Sep 11, 2026

Copy link
Copy Markdown

Concurrent metadata cache misses can delete IDSMetadata.__setattr__ twice and raise AttributeError. The class mutation also temporarily permits writes to metadata belonging to existing IDSs.

Freeze each metadata node when its own initialization finishes, keeping the class setter installed throughout construction. This preserves the constructor's assignments and documented attribute definitions. Build the node type map locally before publishing it, so concurrent first use cannot observe missing numeric array types. The metadata cache and read-only attribute errors are preserved.

Fixes #130.

The guarantee is limited to removing these metadata-construction races. It does not provide general thread safety, concurrent access to a shared IDS, or thread safety for IMAS-Core put, get, serialize, or deserialize. These limits are documented alongside the metadata API and in the changelog.

Tests pause construction at a child node with events while a second IDS is created through IDSFactory, covering different IDS types and concurrent misses for the same cache key. They also check existing metadata during construction, immutability after both builds, failed construction, and overlapping type-map initialization. Waits are bounded and paused workers are released in finally blocks. Four independently populated core_profiles IDSs are validated and compared with sequential results for DD 3.39.0 and 4.0.0.

Validation at 6994382ab643df2085fb9116fc020c9c1c9be0d2:

  • Linux CI, Python 3.10–3.13: 1804 passed, 297 skipped on each version.
  • macOS arm64, Python 3.12: 1804 passed, 297 skipped, including branch coverage. All added executable lines and both branches of the new setter guard were covered.
  • Metadata and hash tests: 38 passed on each of Python 3.10, 3.11, 3.12, and 3.13 on macOS.
  • Metadata tests repeated 50 times in one process: 650 passed.
  • On unmodified develop (4390fae), the four race regressions fail with two AttributeErrors, a missing read-only exception, and a type-map KeyError.
  • Black 24 and Flake8, git diff --check, distribution build, local Sphinx build, and Read the Docs passed. The local wheel contains the exact committed implementation and tests.

A local CPython 3.12 microbenchmark measured uncached core_profiles metadata construction at 5.30 ms before and 7.07 ms after this change; cache hits were approximately 0.035 µs in both cases. These are medians of 15 alternating samples with 20 uncached builds per sample, using DD 3.39.0 and already-parsed XML/path caches. Per-instance freezing adds initialization work; it does not affect the cached construction path.

One line in the existing string-array hash test now encodes expected input as UTF-8. Its previous raw-string input fails with xxhash 4 on unmodified develop; production hashing already encodes strings, and the expected digest is unchanged.

Upstream GitHub Actions runs require maintainer approval for this external contribution. The successful fork runs linked above tested the exact PR commit with the repository's unchanged workflows.

Freeze metadata per instance instead of removing the class setter, and publish the node type map only after construction. Add deterministic concurrent construction and immutability regressions, independent IDS filling checks, and document the limited thread-safety scope.

Encode strings in the existing hash test so the suite also runs with xxhash 4.
@chupakobra6
chupakobra6 marked this pull request as ready for review September 11, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Writing fields on multiple IDSs might not be thread safe

1 participant