Repository navigation
Keep the root of reflected user data alive for its nested items (0125) - #498
Merged
Merged
Conversation
A callable returning reflected data (Reflect of a vector of vectors, of a struct, of a vector of structs) gave nested containers whose accessors are raw pointers into the root; once such an item escaped the result (a loop target kept in a namespace, a sorted or sliced copy) the root was freed and the item dangled. The generic adapters now rewrap nested GenericList and GenericMap items of an owned root so their accessor provider shares the root's ownership. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017GtWzPJSunuQpJtMvc1LFq
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017GtWzPJSunuQpJtMvc1LFq
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
2 participants
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.
Requested by Ruslan · project thread
Before: a user callable returning reflected data with nested containers (
Reflect(std::vector<std::vector<std::string>>), a reflected struct with astd::vector<std::string>field,Reflect(std::vector<Rec>)) crashed with heap-use-after-free under ASan once a nested item outlived the call result, e.g.{% set r = rnested() | sort %}{{ r }}, a loop target kept in a namespace,rrec().copy(),rrecs()[1:].After: those templates render. The nested items keep the callable's result alive.
Nested items of reflected data come from
Reflect(*p), so their accessors are raw pointers into the parent (PtrItemAccessor,ReflectedMapImplover a pointer). When a generic adapter owns its root (BySharedVal, i.e. a callable result), the item outlived the root.How:
GenericListAdapter(indexing, enumerator, clones and transfers) andGenericMapAdapter::GetItemlend items through a newLendGenericItem(owner, Value&&)insrc/internal_value.cpp. When the adapter owns the root, a valid nestedGenericList/GenericMapis rewrapped in an accessor provider that also captures the root'sshared_ptr. Wrapping chains, so deeper nesting stays alive too. Invalid (default-constructed) containers are passed through unwrapped. ByRef roots (render params) are unchanged. The public reflection header is untouched. Strings were already copied on this path.Tests: 11 reflected rows in
OwnedUserDataTest(test/user_callable_test.cpp). They cover the task's templates, attribute chains, sort by attribute, and a record whose fields are invalid containers. The expected strings match Python Jinja2 3.1.6. Task 0125 is marked done.Perf: the bench instruction counts are unchanged (preflight
--perf, all +0.00%). The bench has no reflected nested access, though. On that path each lent nested item now costs one extrastd::functionand holds ashared_ptrcopy.Roles: verifier, 1 round. It found one blocker: an invalid nested container was wrapped into a provider that returned null, and it SEGVed. Fixed in the second commit and pinned by a row. Its ASan probe of 165 templates over reflected sources gave 80 crashes before the fix and 0 after. Red pushes: none so far.
🤖 Generated with Claude Code
https://claude.ai/code/session_017GtWzPJSunuQpJtMvc1LFq