Skip to content

FIX(py/twine): decoder cache leaks across artifacts - #167

Merged
hon-gyu merged 2 commits into
mainfrom
hy/fix-twine-stale-term-leak
Jun 22, 2026
Merged

FIX(py/twine): decoder cache leaks across artifacts#167
hon-gyu merged 2 commits into
mainfrom
hy/fix-twine-stale-term-leak

Conversation

@hon-gyu

@hon-gyu hon-gyu commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

In one of downstream library's tests, the twine decoding gave different results when the test is run isolated vs. in a test suite. It reproduced only with enough prior artifact decodes in the same process to populate the colliding offset.

Summary

twine.Decoder.caches is declared as a class-level mutable dict and is never
re-initialized in __init__, so every Decoder instance shares the same cache
object. Because cached values are keyed by byte-offset within a single artifact's
buffer, offsets from one artifact collide with offsets from a previously-decoded
artifact, and the decoder returns stale decoded values that belong to a different
artifact.

Reproduction

from imandrax_api.lib import twine

d1 = twine.Decoder(b"\x00")
d2 = twine.Decoder(b"\x00")
assert d1.caches is d2.caches           # BUG: same object

d1.caches.setdefault("T", {})[42] = "STALE"
print(d2.caches["T"][42])   # -> "STALE": d2 sees d1's cache

Fix

init cache per Decoder instance (per artifact)

@hon-gyu
hon-gyu requested a review from wintersteiger June 22, 2026 17:11

@wintersteiger wintersteiger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦
LGTM, thanks for the fix! Perhaps we should just remove this cache, and perhaps some others too... users will cache their artifact by themselves if they think there is a need.

@hon-gyu
hon-gyu merged commit 964f736 into main Jun 22, 2026
6 checks passed
@hon-gyu
hon-gyu deleted the hy/fix-twine-stale-term-leak branch June 22, 2026 18:47
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.

2 participants