Identity exposure through GetAssetInfo - #13
Merged
Conversation
A reader can be destroyed arbitrarily late. The resolver retains the reader `Resolve` opened, for the `OpenAsset` that usually follows and is not required to (RESOLVER.md §2.3) -- a host probing for existence resolves constantly and opens rarely -- and those readers are destroyed with the resolver, during static destruction. `~ReaderMetrics` folds into the process aggregate from there, and the aggregate is a function-local static constructed at the *first* reader, which is to say after the resolver that owns the last one. Reverse-order destruction then destroys the aggregate first and the fold reaches it anyway. The existing defence was construction order: `ReaderMetrics`'s constructor touches the registry so that the registry is constructed first. That is the right trick for a `ReaderMetrics` with static storage duration, and it cannot help here, because the reader is not a static -- it is owned by one, and it is destroyed when that static is, which is later than anything the ordering rule covers. So the aggregate is now never destroyed. The leak is the whole point: a counter folded into an immortal aggregate at an arbitrary moment during teardown is correct, and there is no moment at which the fold is unsafe. The opt-in dump moves from the destructor to `std::atexit`, armed at first use, which is the same moment the destructor used to run and therefore the same set of counters: a reader that outlives the handler is missing from the dump, as it was before. Missing a counter in an opt-in diagnostic is a cost; reaching a destroyed aggregate is not a cost, it is a segmentation fault. Reachable since `v0.2.0`, and invisible until now because no test left a retained open behind at exit. It presents as a crash *after* the suite prints "ok", which is the least debuggable moment a process has. The regression test arrives with the identity work that surfaced it, and it asserts nothing in a CHECK: it resolves an asset it never opens, and the exit code does the asserting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`GetAssetInfo` now answers with the four neutral values RESOLVER.md §3 fixes -- `resolvedIdentifier`, `size`, `validationToken`, `stability` -- in `ArAssetInfo::resolverInfo`, under the names the first consumer's own contract uses. The token is the backend's captured validator, opaque here and opaque all the way out; `stability` is the only projection of validator strength that crosses this boundary, and `ValidatorKind` and `ValidatorStrength` stay below it where ASSET_READER.md §7.1 puts them. What decided the shape is not in this repository. The consumer reads `ArAssetInfo::version`, falls back to `GetModificationTimestamp`, and classifies the result itself: a non-blank identifier plus a non-blank token is `Stable`. It consults no stability field. So the fail-safe cannot be carried by a stability value beside the token -- in the field a consumer actually reads, nothing is beside it. Three consequences, and each is a rule rather than a detail: - `version` carries the token only for a `Stable` identity. A weak validator publishes its token in the annotated dictionary instead, where `stability` stands next to it, because a weak validator that *changed* is still evidence the asset changed (§7.2); it is proof of sameness that weak cannot give. - `GetModificationTimestamp` is invalid, permanently, and is now overridden rather than inherited so that the reasoning sits where the tempting mistake would be made. A validator is not a time, reading `Last-Modified` as one parses an HTTP construct above the backend that captured it, and a fabricated number would be turned into a `resolver-mtime:` identity by that same consumer fallback -- manufacturing a durable identity for an asset that has none. - An identifier whose two opens captured two different validators stops publishing a reusable identity for the rest of the process. `ArAssetInfo` is keyed by path, this resolver hands out one reader per open, and after a republish there is no way to say which revision a caller holds; a consumer given the newer token for older bytes would file revision A under the identity of revision B, and nothing downstream could detect it. `Stable` degrades to `Unstable`, `version` empties, and the token stays visible as evidence of change. The contradiction is remembered permanently -- a bounded table would forget it and start publishing reusability again for an asset that has already proved it has none. The identity is the open's, not a fresh request's. Asset info is answered from the open this process already performed, which is cheaper and, more to the point, more correct: what a consumer needs is the identity of the bytes it is holding, and a `HEAD` issued now describes whatever is published now. An identifier nothing has opened is opened here and retained through the same table `_Resolve` fills, so the request costs what the `OpenAsset` after it would have. `Identity.cpp` is a separate translation unit with no OpenUSD header, for the reason `Identifier.cpp` is: the mistakes it can make are silent. A token published for a validator that cannot prove a revision does not fail here, it fails in a consumer, weeks later, as a generated cache that served the wrong bytes -- and by then the resolver looks correct, because every read it performed was correct. `httpResolver_identity` asserts the rules as a table. `httpResolver_stage` adds seven cases over a real socket, and one of them asserts nothing in a CHECK: it resolves an asset it never opens, leaving a retained reader to be destroyed during static teardown, and what it asserts is the exit code. That is the case for the crash fixed in the commit before this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The code for identity exposure landed in the commit before this one; what is left is every document that had said the surface was planned, and one question in an ADR that the implementation answered. RESOLVER.md §3 loses its "Planned" heading and gains the five subsections the work produced: why `version` is strong-only while the dictionary may carry a weak token, what a contradicted identifier does, why the identity is the open's rather than a new request's, why `GetModificationTimestamp` declines to answer, and what elision costs. That last one is worth stating rather than implying: two assets that differ only in their query strings elide to one `resolvedIdentifier`, so that field is not by itself a cache key -- the token is what distinguishes revisions, and a consumer that wants the URL it asked about already holds it. consumer-integration.md §4.1 records how the consumer actually reads this, because a rule whose reason lives in another repository is a rule somebody will relax. It reads `version`, falls back to the timestamp, and classifies the result itself with no stability field consulted. Everything strict about the surface follows from that, and the consumer needs no change for any of it -- which is criterion 1 of §7 holding rather than being asserted. ADR-0001's second open question is answered and the ADR stands: identity belongs in `GetAssetInfo`, and the surface is weak in exactly one way that had to be designed around rather than routed around -- it is keyed by path, not by open asset, so it cannot say which of two revisions a caller holds. The rest is bookkeeping the invariants require: - CAPABILITY_MATRIX.md moves asset info to implemented and adds the four rows that state what implemented *means* here: the `version` rule, the republish rule, a permanently invalid timestamp, and the aggregate that survives a reader destroyed during teardown. - implementation-status.md records that `v0.3.0` is released, splits phase 4 into the half that has landed and the half that has not, and names the next work as persistence with CACHE.md §8's requirement list rather than as "persistence". - The bundle README gains an *Asset info and identity* section, and loses two claims that `v0.3.0` had already made false: that the bundle performs no caching, and a configuration table listing five variables out of nine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four defects found in review of this branch, and the first two are the kind that publish a wrong identity rather than fail. **An aged-out answer must not age out the question.** One bounded table held both the metadata that lets asset info skip a request and the validator a later open is compared against, so past 512 identifiers the eviction that was supposed to cost a metadata request cost a correctness property instead: an asset that had already been republished looked like an asset being opened for the first time, and published a reusable token for a revision a live consumer was not holding. The comment defended the bound by pointing at the permanent set of contradicted identifiers, which only preserves contradictions already observed -- not the ones eviction stops from ever being observable. The two records are now separate structures with separate lifetimes. `_identities` stays bounded and caches an answer, because dropping one costs a request and nothing else. `_fingerprints` is per identifier, kept for the life of the process, and holds the validator plus whether this identifier has ever been seen with two: it is the memory a change is noticed against, and a bound on it is a bound on how far back a republish can be noticed. It costs a URL and an ETag per asset the process has actually opened, which is the same order as the identifiers a host is holding anyway; the expensive half is the metadata, and the metadata is the half that is still bounded. **A reader that leaves is not an open that failed.** `_OpenAsset` moves the reader out of an entry it took, leaving `opened` true, `reader` null, and the status `Ok`. A concurrent `_IdentityFor` holding the same entry read that as a failed open and returned a default-constructed `ArAssetInfo` for an asset that had opened perfectly well. `_Opened` now copies the metadata off the reader at open and records that it succeeded, so handing the reader out cannot erase what the open discovered; `_Resolve` answers from the same field, which fixes the same race in the resolution path -- it returned an empty path for an existing asset whose reader another thread had just taken. **Asset info is not the call that should discover a dead origin.** It issued its own metadata request for a path that had not resolved and posted its own `HTTPxxx`, so a `SdfLayer::Reload` against a `503` cost two round trips and printed one fault twice. It now declines to open an identifier whose resolved path is empty -- that is a resolution that already failed, and `_Resolve` has just paid for the discovery -- and it posts nothing at all: identity is a question about the asset, the operation that follows reports the same fault with the same code, and DIAGNOSTICS.md §3 wants a fault reported once. **The changelog undercounted its own cases**, and did not list the credential elision one. It now names ten, including the three added here. The two correctness fixes have cases: a republish detected after 520 other identifiers have pushed the first one out of the bounded table -- which fails against the previous commit, checked by reintroducing the old eviction -- and eight threads resolving, asking for identity, and opening one asset, asserting that no thread ever saw an asset with no identity or no `ArAsset`. Those cases count requests per asset out of the fixture server's log rather than from its total, and so does `TestResolveIsNotRepeated`, which had the flaw first: a case that abandons a response mid-body -- the range-unsupported one does, by contract -- leaves the server writing to a socket nobody is reading, and that request can be logged inside a later case's window. It presented once as `TestAssetInfoIsPublished` seeing a request it had not made. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
snkmcb
added a commit
that referenced
this pull request
Aug 22, 2026
VERSION, openstrata.toml, the plugin manifest, and the bundle's standalone CMake fallback move to 0.4.0; the Unreleased changelog section is finalized and points at docs/releases/v0.4.0.md, the immutable record, which the index now links. The record's gate 2 cites both PRs rather than one: this release is #13 and #14, green on 5f7966f and 917c7dc with 12 checks each. Gate 7 answers a question no earlier record had to -- a cache entry is the first artifact this project persists, and it is the one place ElideSecrets cannot run, so the row says why the key is a digest and never the identifier. The root README's Status had drifted a release behind: it still announced v0.2.0 as released and v0.3.0 as unreleased. It now states v0.4.0 and says what the two halves of it buy a reader, and the build section no longer says "both releases so far". The capability matrix's body was updated by #13 and #14; only its Last updated stamp was stale, and it is now the tree it describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first half of
v0.4.0: identity leaves the process. The other half —on-disk persistence — is not in this PR.
What ships
GetAssetInfopublishes the four neutral values ofRESOLVER.md §3 in
ArAssetInfo::resolverInfo:resolvedIdentifier,size,validationToken,stability. The token is thebackend's captured validator, opaque here and opaque all the way out.
What decided the shape of the surface is in the consumer, not here.
usd-pointcloud-pluginsreadsArAssetInfo::version, falls back toGetModificationTimestamp, and classifies the result itself — a non-blankidentifier plus a non-blank token is
Stable— consulting no stability field.The fail-safe therefore cannot ride next to the token, because in the field a
consumer actually reads, nothing rides next to it. Three rules follow:
versioncarries a token only for aStableidentity. A weak validator'stoken goes in the annotated dictionary, where
stabilitystands beside it: aweak validator that changed is still evidence the asset changed, and it is
proof of sameness that weak cannot give.
GetModificationTimestampis invalid, permanently, and now overriddenrather than inherited so the reasoning sits where the tempting mistake would
be made. A fabricated time would be turned into a
resolver-mtime:identityby that same consumer fallback — manufacturing a durable identity for an asset
that has none.
ArAssetInfois keyed bypath and this resolver hands out one reader per open, so after a republish
underneath a live process there is no way to say which revision a caller
holds.
Stabledegrades toUnstableandversionempties for the rest ofthe process; the current token stays visible as evidence of change.
Asset info is answered from the open this process already performed, never from
a metadata request issued to answer the question — cheaper, and more to the
point more correct: the identity a consumer needs is the identity of the bytes
it is holding, and a
HEADissued now describes whatever is published now.A crash this surfaced, fixed here
A reader retained by
Resolveand never opened is destroyed with the resolver,during static destruction, and folded its counters into a process metrics
aggregate that had been destroyed first — the aggregate is constructed at the
first reader and so is destroyed before the resolver constructed before it.
Reachable since
v0.2.0by any host that probes for existence and exits;invisible until a test left a retained open behind. It presents as a
segmentation fault after the suite prints
ok.The aggregate is now never destroyed and the opt-in
USD_HTTP_RESOLVER_METRICS_DUMPruns fromstd::atexit. The regression testasserts nothing in a
CHECK: it resolves an asset it never opens, and the exitcode does the asserting.
Tests
httpResolver_identity, new, offline, one translation unit: strong, weak,Last-Modified, absent, contradicted, a strength with no kind, and bothcredential shapes. The defect it exists to catch does not fail here — it fails
in a consumer, weeks later, as a generated cache that served the wrong bytes.
httpResolver_stage, seven new cases over a real socket: the four fields, theversionrule per stability class, a republish that withdraws reusability, aninvalid timestamp, an identity that costs no second metadata request, and the
teardown case above.
core-asan(address,undefined, GCC 15.2)core-tsan(thread, GCC 15.2)Documents
RESOLVER.md §3 rewritten and no longer "Planned"; consumer-integration.md §4.1
records how the consumer reads the surface, because a rule whose reason lives in
another repository is a rule somebody will relax; ADR-0001's second open question
answered, ADR stands. CAPABILITY_MATRIX, WORKSPACE, implementation-status, the
roadmap, the bundle README, and CHANGELOG follow. The README also loses two
claims
v0.3.0had already made false: that the bundle performs no caching, anda configuration table listing five variables out of nine.
🤖 Generated with Claude Code