OpenChemistry / avogadrolibs

Avogadro libraries provide 3D rendering, visualization, analysis and data processing useful in computational chemistry, molecular modeling, bioinformatics, materials science, and related areas.
https://two.avogadro.cc/
BSD 3-Clause "New" or "Revised" License
455 stars 172 forks source link

Consistent segfaults while manipulating bonds between independent carbon bonds + hydrogens #1492

Open Makiah opened 11 months ago

Makiah commented 11 months ago

Avogadro version: (please complete the following information from the About box):

Desktop version: (please complete the following information):

Describe the bug Avogadro 2 will segfault and display some suboptimal behavior while manipulating bonds and hydrogens between two independent carbon atoms. I've narrowed one potential problematic sequence of events down to the following, for which I've attached screenshots in order to reproduce.

To Reproduce

  1. Open Avogadro 2, create two carbon atoms by left clicking with the "Draw" tool. step1
  2. Left click on the first carbon atom, and drag the resulting bond to the second carbon atom. You should see the following. Notably this hydrogen manipulation done after the bond manipulation is also incorrect, as the second carbon now has zero attached hydrogens: this seems related to https://github.com/OpenChemistry/avogadrolibs/issues/1478. Screenshot from 2023-12-03 18-55-41
  3. Remove the bond between the first and second carbon by right clicking it. Screenshot from 2023-12-03 18-56-01
  4. Remove the second carbon by right clicking it Screenshot from 2023-12-03 18-56-13
  5. Remove the first carbon by right clicking it. You'll notice a segfault prompted by a nullptr access to Molecule::bonds, I have a quick fix set up locally so I see the following. Screenshot from 2023-12-03 18-56-27 Screenshot from 2023-12-03 18-56-35

Working on understanding the root cause here to ideally address this issue and https://github.com/OpenChemistry/avogadrolibs/issues/1478 simultaneously.

Expected behavior No segfaults, valence shell of carbons correctly populated at each step in the above process (might split this into a couple separate PRs).

Screenshots Attached above

welcome[bot] commented 11 months ago

Thanks for opening your first issue here! Please try to include example files and screenshots if possible. If you're looking for support, please post on our forum: https://discuss.avogadro.cc/

ghutchis commented 9 months ago

Is this now fixed by #1493 ? I was never able to reproduce this, so I need to know if your patch fixed the underlying issue. Thanks.

ghutchis commented 6 hours ago

Once again, I'm pretty sure this is fixed. Can you please check? Otherwise I'm going to close this in a week as resolved.