Skip to content

Fix metadata dependency cache leaking across the MRO - #1468

Open
Sanjays2402 wants to merge 1 commit into
Instagram:mainfrom
Sanjays2402:fix/metadata-dependency-cache-mro
Open

Fix metadata dependency cache leaking across the MRO#1468
Sanjays2402 wants to merge 1 commit into
Instagram:mainfrom
Sanjays2402:fix/metadata-dependency-cache-mro

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #1459

Summary

MetadataDependent.get_inherited_dependencies() memoized into a plain class
attribute and read it back with ordinary attribute lookup, which walks the MRO.
Resolving a bare CSTTransformer/CSTVisitor during a metadata pass therefore
cached an empty frozenset onto the base class, and every subclass that had not
yet computed its own cache inherited that empty set — raising a spurious
METADATA_DEPENDENCIES KeyError even though the subclass declared its
provider correctly. The poisoning is process-wide and surfaces far from its
cause.

The fix reads only the class's own __dict__ entry, so a base-class cache can
never be inherited. Memoization and its per-class semantics are unchanged.

Test Plan

Added test_bare_base_class_does_not_poison_inherited_metadata in
libcst/metadata/tests/test_metadata_provider.py, beside the existing
test_inherited_metadata: it resolves a bare CSTTransformer first, then a
subclass declaring a provider. Stash-verified — the test fails without the
source change (the exact reported KeyError) and passes with it.
libcst/metadata/tests/ is 174 passed; the 3 errors are the pre-existing pyre
test_type_inference_provider cases that need the pyre binary.

This change was prepared with AI assistance; the regression test was run locally
and fails without the fix.

get_inherited_dependencies() memoized into a plain class attribute and
read it back with ordinary attribute lookup, which walks the MRO. Running
a metadata pass with a bare CSTTransformer/CSTVisitor therefore cached an
empty frozenset on the base class, and every subclass that had not yet
computed its own cache inherited that empty set, raising a spurious
METADATA_DEPENDENCIES KeyError.

Read only the class's own __dict__ entry so a base-class cache can never
be inherited. Memoization and its per-class semantics are unchanged.

Adds a regression test that resolves a bare CSTTransformer before a
subclass declaring a provider.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 6, 2026
@Sanjays2402

Copy link
Copy Markdown
Author

Ping on this one — CI is blocked on the first-run approval gate. Note this supersedes #1466 (now closed), which fixed the same issue with a larger diff. Happy to adjust if you'd prefer a different approach to #1459.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bare CSTTransformer metadata pass poisons the inherited-dependency cache for all later subclasses

1 participant