Skip to content

Fix potential iterator invalidation#8944

Merged
greglandrum merged 1 commit intordkit:masterfrom
rietmann-nv:rietmann-nv/iterator-invalidation
Nov 11, 2025
Merged

Fix potential iterator invalidation#8944
greglandrum merged 1 commit intordkit:masterfrom
rietmann-nv:rietmann-nv/iterator-invalidation

Conversation

@rietmann-nv
Copy link
Copy Markdown
Contributor

The iterator in the loop over bonds (changed in utils.cpp) is potentially invalidated by adding bonds to the molecule. We copy the bonds into a vector to ensure safety.

What does this implement/fix? Explain your changes.

We found an iterator invalidation when re-writing the backend and decided to port this over to the existing codebase in anticipation of future merges.

The iterator in the loop over bonds (changed in utils.cpp) is potentially
invalidated by adding bonds to the molecule. We copy the bonds into a vector to
ensure safety.
Copy link
Copy Markdown
Member

@greglandrum greglandrum left a comment

Choose a reason for hiding this comment

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

LGTM

@greglandrum greglandrum added this to the 2025_09_3 milestone Nov 11, 2025
@greglandrum
Copy link
Copy Markdown
Member

@rietmann-nv thanks for the contribution!
I will merge this after the CI builds go through.

@greglandrum greglandrum merged commit 3802eaa into rdkit:master Nov 11, 2025
10 of 12 checks passed
greglandrum pushed a commit that referenced this pull request Nov 28, 2025
The iterator in the loop over bonds (changed in utils.cpp) is potentially
invalidated by adding bonds to the molecule. We copy the bonds into a vector to
ensure safety.
pechersky pushed a commit to pechersky/rdkit that referenced this pull request Jan 7, 2026
The iterator in the loop over bonds (changed in utils.cpp) is potentially
invalidated by adding bonds to the molecule. We copy the bonds into a vector to
ensure safety.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants