Segfault when accessing m_vibrations in vibrationdialog.cpp

Development
#1

Hi all – To introduce myself, I’m a chemistry undergrad at Austin
Peay State University in Tennessee, and I’ll be starting in the
University of Buffalo’s Ph.D. program this fall. I’m trying to get
involved in avogadro, and I’ve been hanging around in IRC lately as
Mr_Grieves. I’m more comfortable with python than c++, although I’m
slowly adjusting to it.

Enough of that :), here’s my problem:

I’m adding a tsv export to the vibrations dialog, but the member
pointer m_vibrations is becoming invalid by the time I access it to
get the vibration data. The attached patch’s copious debug output
tests the number of vibrations to show this problem. The program
segfaults when trying to retrieve the frequency data. Any ideas of how
to get around this?

Thanks,

Dave

#2

On Mar 4, 2009, at 9:42 AM, David Lonie wrote:

I’m adding a tsv export to the vibrations dialog, but the member
pointer m_vibrations is becoming invalid by the time I access it to
get the vibration data. The attached patch’s copious debug output
tests the number of vibrations to show this problem. The program
segfaults when trying to retrieve the frequency data. Any ideas of how
to get around this?

Out of curiosity, what platform are you using? And what version of the
Avogadro code are you using?

Right now, things seem OK on Mac and Linux, but we’re definitely
seeing crashes on Windows similar to your description. So you’re
coming at a good time. :slight_smile:

Cheers,
-Geoff

#3

Hi,

In the original virbariondialog.* from Geoff, m_vibrations was only
used in a single function. It is assigned by the following code

OBMol obmol = m_molecule->OBMol();
m_vibrations =

static_cast<OBVibrationData*>(obmol.GetData(OBGenericDataType::VibrationData));

The VibrationExtension has the same member variable, assignment and
used in the same way. It would be better to have this as a local
pointer and add it as a parameter to
VibrationExtension::updateForcesAndFrames(…)

The reason for the crashes is likely due to the fact that as soon as
we leave the scope where it was assigned, the obmol object is deleted
which also deletes the data.

Tim

On Wed, Mar 4, 2009 at 3:42 PM, David Lonie loniedavid@gmail.com wrote:

Hi all – To introduce myself, I’m a chemistry undergrad at Austin
Peay State University in Tennessee, and I’ll be starting in the
University of Buffalo’s Ph.D. program this fall. I’m trying to get
involved in avogadro, and I’ve been hanging around in IRC lately as
Mr_Grieves. I’m more comfortable with python than c++, although I’m
slowly adjusting to it.

Enough of that :), here’s my problem:

I’m adding a tsv export to the vibrations dialog, but the member
pointer m_vibrations is becoming invalid by the time I access it to
get the vibration data. The attached patch’s copious debug output
tests the number of vibrations to show this problem. The program
segfaults when trying to retrieve the frequency data. Any ideas of how
to get around this?

Thanks,

Dave

Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H

Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/avogadro-devel

#4

The reason for the crashes is likely due to the fact that as soon as
we leave the scope where it was assigned, the obmol object is deleted
which also deletes the data.

Yes, you’re correct. I keep forgetting that Molecule::OBMol doesn’t
cache – even though the OBVibrationData is conserved by the Molecule
class.

I’ll clean this up in a few minutes and commit David’s patch.

-Geoff