From 3802eaaac9480c98592ad79e6d5face101a9d574 Mon Sep 17 00:00:00 2001 From: Max Rietmann <47424406+rietmann-nv@users.noreply.github.com> Date: Tue, 11 Nov 2025 17:03:05 +0100 Subject: [PATCH] Fix potential iterator invalidation (#8944) 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. --- External/ChemDraw/utils.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/External/ChemDraw/utils.cpp b/External/ChemDraw/utils.cpp index 1b2b83daa..ea979a014 100644 --- a/External/ChemDraw/utils.cpp +++ b/External/ChemDraw/utils.cpp @@ -108,8 +108,16 @@ struct FragmentReplacement { auto bond_ordering = replacement_atom->getProp>(CDX_BOND_ORDERING); + // The "addBond" lower in the loop potentially modifies the atomBonds + // iterator. To ensure safety, we copy the bonds first. + std::vector replacement_bonds( + mol.atomBonds(replacement_atom).begin(), + mol.atomBonds(replacement_atom).end()); + + std::vector xbonds; // Reuse this vector to reduce repeated allocations + // Find the connecting atoms and and do the replacement - for (auto bond : mol.atomBonds(replacement_atom)) { + for (auto bond : replacement_bonds) { // find the position of the attachment bonds in the bond ordering unsigned bond_id = 0; if (!bond->getPropIfPresent(CDX_BOND_ID, bond_id)) { @@ -134,7 +142,12 @@ struct FragmentReplacement { auto &xatom = fragment_atoms[pos]; - for (auto &xbond : mol.atomBonds(xatom)) { + // The "addBond" lower in the loop potentially modifies the atomBonds + // iterator. To ensure safety, we copy the bonds first. + xbonds.assign(mol.atomBonds(xatom).begin(), + mol.atomBonds(xatom).end()); + + for (auto &xbond : xbonds) { // xatom is the fragment dummy atom // xbond is the fragment bond if (bond->getBeginAtom() == replacement_atom) {