Skip to content

Conversation

@hageboeck
Copy link
Member

This is a followup of the fix #20352 for issue #20320:
When snapshot with variations is run with JIT-ted filters, the
computation graph first needs to be JIT-ted before reading Defines
or Filters. In the fix for that case, an iterator invalidation on
push_back caused crashes, because vector.reserve() had been called
with the wrong size.

Here, the size is corrected, but more importantly, instead of saving the
iterator, the shared_ptr it points to is saved. This makes the code
independent of the underlying storage.

The problem was originally described here:
#20320 (comment)
The code posted there has been converted into a test, and added to this PR.

This is a followup of the fix root-project#20352 for issue root-project#20320:
When snapshot with variations is run with JIT-ted filters, the
computation graph first needs to be JIT-ted before reading Defines
or Filters. In the fix for that case, an iterator invalidation on
push_back caused crashes, because vector.reserve() had been called
with the wrong size.

Here, the size is corrected, but more importantly, instead of saving the
iterator, the shared_ptr it points to is saved. This makes the code
independent of the underlying storage.
@hageboeck hageboeck force-pushed the RDF_SnapshotVarCrash branch from 3bdfe7b to e57f7e2 Compare November 24, 2025 15:51
Copy link
Contributor

@martamaja10 martamaja10 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Stephan!

@github-actions
Copy link

Test Results

    22 files      22 suites   3d 21h 12m 45s ⏱️
 3 782 tests  3 782 ✅ 0 💤 0 ❌
81 210 runs  81 210 ✅ 0 💤 0 ❌

Results for commit e57f7e2.

@hageboeck hageboeck merged commit c7b4143 into root-project:master Nov 25, 2025
30 checks passed
@hageboeck hageboeck deleted the RDF_SnapshotVarCrash branch November 25, 2025 08:03
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