Thread problems during moleculeChanged calls

[cryos]

When calling moleculeChanged to update the mainwindow's widget, I will frequently get one of the following:

ASSERT failure in QReadLocker: "QReadWriteLock pointer is misaligned", file /usr/include/QtCore/qreadwritelock.h, line 132

QMutex::lock: mutex lock failure:

or sometimes just a segfault. It happens during the following call to moleculeChanged -- the debug at the begin appears, but not the last. m_results_list is a QList<Xtal*>*, where Xtal is a class derived from Avogadro::Molecule.

void XtalOptDialog::selectMoleculeFromResults(int row,int,int oldrow,int) {
qDebug() << "XtalOptDialog::selectMoleculeFromResults( " << row << " " << oldrow << " ) called";
if (row == oldrow) return;
emit moleculeChanged(m_results_list->at(row));
qDebug() << "molecule set!";
}

This is happening with the code at this revision of the xtalopt extension: http://github.com/dlonie/avogadro-XtalOpt/commit/a304f765bd2ace3cd42f03af2c9a9946cf1a1f25 . The snippet above comes from the bottom of xtaloptdialog.cpp

Reported by: @dlonie

[cryos]

Backtrace for the ASSERT failure:

#0 0xb7fba424 in __kernel_vsyscall ()
#1 0xb7e7e471 in raise () from /lib/libc.so.6
#2 0xb7e7fca4 in abort () from /lib/libc.so.6
#3 0xb6cfb0bd in qt_message_output () from /usr/bin/../lib/libQtCore.so.4
#4 0xb6cfb1a1 in qFatal () from /usr/bin/../lib/libQtCore.so.4
#5 0xb6cfb24c in qt_assert_x () from /usr/bin/../lib/libQtCore.so.4
#6 0xb6a666b6 in QReadLocker (this=0xbfcd35cc, areadWriteLock=0xffffffff) at /usr/include/QtCore/qreadwritelock.h:132
#7 0xb6adf104 in Avogadro::Molecule::fileName (this=0x93a0578) at /usr/src/avogadro/libavogadro/src/molecule.cpp:130
#8 0x0809611b in Avogadro::MainWindow::setMolecule (this=0x867b828, molecule=0x93a0578) at /usr/src/avogadro/avogadro/src/mainwindow.cpp:2136
#9 0x0809b9b6 in Avogadro::MainWindow::qt_metacall (this=0x867b828, _c=QMetaObject::InvokeMetaMethod, _id=62, _a=0xbfcd3728)
at /usr/src/avogadro/build/avogadro/src/mainwindow.moc:240
#10 0xb6e0c339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#11 0xb6e0cfa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#12 0xb6b05c88 in Avogadro::Extension::moleculeChanged (this=0x8c47470, _t1=0x93a0578) at /usr/src/avogadro/build/libavogadro/src/extension.moc:103
#13 0xb50e917a in Avogadro::XtalOptExtension::reemitMoleculeChanged () from /usr/lib/avogadro/contrib/xtalopt.so
#14 0xb50e93d6 in Avogadro::XtalOptExtension::qt_metacall () from /usr/lib/avogadro/contrib/xtalopt.so
#15 0xb6e0c339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#16 0xb6e0cfa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#17 0xb50edc76 in Avogadro::XtalOptDialog::moleculeChanged () from /usr/lib/avogadro/contrib/xtalopt.so
#18 0xb50ed9c4 in Avogadro::XtalOptDialog::selectMoleculeFromResults () from /usr/lib/avogadro/contrib/xtalopt.so
#19 0xb50edc1d in Avogadro::XtalOptDialog::qt_metacall () from /usr/lib/avogadro/contrib/xtalopt.so
#20 0xb6e0c339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#21 0xb6e0cfa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#22 0xb78167a5 in QTableWidget::currentCellChanged () from /usr/bin/../lib/libQtGui.so.4
#23 0xb781d8b0 in ?? () from /usr/bin/../lib/libQtGui.so.4
#24 0xb781eac9 in QTableWidget::qt_metacall () from /usr/bin/../lib/libQtGui.so.4
#25 0xb6e0c339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#26 0xb6e0cfa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#27 0xb77fd249 in QItemSelectionModel::currentChanged () from /usr/bin/../lib/libQtGui.so.4
#28 0xb77fd3ed in QItemSelectionModel::setCurrentIndex () from /usr/bin/../lib/libQtGui.so.4
#29 0xb77aefa6 in QAbstractItemView::mousePressEvent () from /usr/bin/../lib/libQtGui.so.4
#30 0xb728dc86 in QWidget::event () from /usr/bin/../lib/libQtGui.so.4
#31 0xb76623d3 in QFrame::event () from /usr/bin/../lib/libQtGui.so.4
#32 0xb7701d2f in QAbstractScrollArea::viewportEvent () from /usr/bin/../lib/libQtGui.so.4
#33 0xb77b33f7 in QAbstractItemView::viewportEvent () from /usr/bin/../lib/libQtGui.so.4
#34 0xb7704325 in ?? () from /usr/bin/../lib/libQtGui.so.4
#35 0xb6df546a in QCoreApplicationPrivate::sendThroughObjectEventFilters () from /usr/bin/../lib/libQtCore.so.4
#36 0xb72376ca in QApplicationPrivate::notify_helper () from /usr/bin/../lib/libQtGui.so.4
#37 0xb72402bf in QApplication::notify () from /usr/bin/../lib/libQtGui.so.4

Original comment by: @dlonie

[cryos]

Backtrace for the segfault:

#0 0xb6e00fa2 in QMutex::lock () from /usr/bin/../lib/libQtCore.so.4
#1 0xb6e01e50 in QReadWriteLock::lockForRead () from /usr/bin/../lib/libQtCore.so.4
#2 0xb6c665f0 in Avogadro::Molecule::fileName (this=0x92d4440) at /usr/include/QtCore/qreadwritelock.h:111
#3 0x08091a23 in Avogadro::MainWindow::setMolecule (this=0x84bbb50, molecule=0x92d4440) at /usr/src/avogadro/avogadro/src/mainwindow.cpp:2135
#4 0x080971ca in Avogadro::MainWindow::qt_metacall (this=0x84bbb50, _c=QMetaObject::InvokeMetaMethod, _id=62, _a=0xbfcd5ee8)
at /usr/src/avogadro/build/avogadro/src/mainwindow.moc:240
#5 0xb6f0f339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#6 0xb6f0ffa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#7 0xb6c78323 in Avogadro::Extension::moleculeChanged (this=0x8a93c80, _t1=0x92d4440) at /usr/src/avogadro/build/libavogadro/src/extension.moc:103
#8 0xb545d86a in Avogadro::XtalOptExtension::reemitMoleculeChanged () from /usr/lib/avogadro/contrib/xtalopt.so
#9 0xb545dac6 in Avogadro::XtalOptExtension::qt_metacall () from /usr/lib/avogadro/contrib/xtalopt.so
#10 0xb6f0f339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#11 0xb6f0ffa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#12 0xb54626a4 in Avogadro::XtalOptDialog::moleculeChanged () from /usr/lib/avogadro/contrib/xtalopt.so
#13 0xb54621f1 in Avogadro::XtalOptDialog::selectMoleculeFromResults () from /usr/lib/avogadro/contrib/xtalopt.so
#14 0xb546264b in Avogadro::XtalOptDialog::qt_metacall () from /usr/lib/avogadro/contrib/xtalopt.so
#15 0xb6f0f339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#16 0xb6f0ffa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#17 0xb79197a5 in QTableWidget::currentCellChanged () from /usr/bin/../lib/libQtGui.so.4
#18 0xb79208b0 in ?? () from /usr/bin/../lib/libQtGui.so.4
#19 0xb7921ac9 in QTableWidget::qt_metacall () from /usr/bin/../lib/libQtGui.so.4
#20 0xb6f0f339 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#21 0xb6f0ffa2 in QMetaObject::activate () from /usr/bin/../lib/libQtCore.so.4
#22 0xb7900249 in QItemSelectionModel::currentChanged () from /usr/bin/../lib/libQtGui.so.4
#23 0xb79003ed in QItemSelectionModel::setCurrentIndex () from /usr/bin/../lib/libQtGui.so.4
#24 0xb78b1fa6 in QAbstractItemView::mousePressEvent () from /usr/bin/../lib/libQtGui.so.4
#25 0xb7390c86 in QWidget::event () from /usr/bin/../lib/libQtGui.so.4
#26 0xb77653d3 in QFrame::event () from /usr/bin/../lib/libQtGui.so.4
#27 0xb7804d2f in QAbstractScrollArea::viewportEvent () from /usr/bin/../lib/libQtGui.so.4
#28 0xb78b63f7 in QAbstractItemView::viewportEvent () from /usr/bin/../lib/libQtGui.so.4
#29 0xb7807325 in ?? () from /usr/bin/../lib/libQtGui.so.4
#30 0xb6ef846a in QCoreApplicationPrivate::sendThroughObjectEventFilters () from /usr/bin/../lib/libQtCore.so.4
#31 0xb733a6ca in QApplicationPrivate::notify_helper () from /usr/bin/../lib/libQtGui.so.4
#32 0xb73432bf in QApplication::notify () from /usr/bin/../lib/libQtGui.so.4
#33 0xb6ef924b in QCoreApplication::notifyInternal () from /usr/bin/../lib/libQtCore.so.4
#34 0xb73423b3 in QApplicationPrivate::sendMouseEvent () from /usr/bin/../lib/libQtGui.so.4
#35 0xb73b1423 in ?? () from /usr/bin/../lib/libQtGui.so.4
#36 0xb73b0906 in QApplication::x11ProcessEvent () from /usr/bin/../lib/libQtGui.so.4
#37 0xb73d9b62 in ?? () from /usr/bin/../lib/libQtGui.so.4

Original comment by: @dlonie

[cryos]

This is actually an issue with the MainWindow::setMolecule(Molecule *) slot. If it has a Molecule object pointer already, it will delete that before setting the new molecule. For all the other code we wrote this behaviour was fine and did what we expected. As your code holds onto the molecules and may use them again, it segfaults the second time the molecule is set as you are using a dangling pointer.

I will fix this tomorrow probably. I think the best solution is to add a second parameter (enum possibly) with options. Such as DeleteOldMolecule, OpenInNewWindow. There is another case where is the window has a loaded molecule in it, then I would like it to open a new window rather than overwrite the active molecule. In your case you do not want that behaviour. There may be other permutations of this too.

Original comment by: @cryos

[cryos]

I fixed this, but forgot to close the bug report. It appears to be working well now.

Original comment by: @cryos