fix: key lifecycle state by Click context - #399
codeforester wants to merge 2 commits into
Conversation
| # Keep the context object itself as the key. ``id(context)`` values can be | ||
| # reused after Click releases a context, which could associate a later | ||
| # invocation with stale lifecycle values. | ||
| context_values = captures.setdefault(click_context, {}) |
There was a problem hiding this comment.
Test gap: issue #383's acceptance criteria explicitly require "a regression test exercises a chain=True group with per-command lifecycle values and asserts each command observes its own resolved values, with the intermediate contexts released between members." The new test (tests/test_click_tree_attachment.py) only exercises _capture_lifecycle_option() with a bespoke FakeContext double, not a real chain=True group with context teardown/GC timing. If a future refactor reintroduces id()-based keying, or this fix is subtly wrong under real Click context lifecycle (vs. the synthetic double), the suite would still pass and the misattribution bug could ship silently again.
| # Keep the context object itself as the key. ``id(context)`` values can be | ||
| # reused after Click releases a context, which could associate a later | ||
| # invocation with stale lifecycle values. | ||
| context_values = captures.setdefault(click_context, {}) |
There was a problem hiding this comment.
Design note: this fix satisfies "no id() reuse" but doesn't address issue #383's other acceptance criterion that "memory behaviour for a long chain is documented or bounded." The capture/resolution dicts still grow one entry per context for the invocation's life, and now pin full Context objects alive (via the shared meta dict) rather than lightweight int keys. A chain=True invocation with a very large number of subcommands now keeps one live Context object per command alive for the whole invocation, with no comment/doc acknowledging or bounding this, despite the issue calling it out as required.
This branch has not been deployed
1 participant
Fixes #383