Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Crash at closing after deleting a hop #116

Closed
theophae opened this issue Nov 10, 2015 · 8 comments
Closed

Crash at closing after deleting a hop #116

theophae opened this issue Nov 10, 2015 · 8 comments
Labels
bug high priority
Milestone
v2.4.0

Comments

@theophae
Copy link
Contributor

This bug is very simple to reproduce:

  • Click on the Hops tab on the left
  • right click and choose "New->Hop"
  • Enter the name of the new hop and click on "OK"
  • The hop editor open, just click on "OK"
  • The new hop is added at the end of the list
  • Right click on the new hop and choose "Delete" (or simply press "Del") and confirm deletion.
  • Close Brewtarget and choose either to save or no the changes and then it cashes
@rocketman768 rocketman768 added this to the v2.3.0 milestone Nov 11, 2015
@rocketman768
Copy link
Member 

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff781ff73 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
(gdb) bt
#0  0x00007ffff781ff73 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#1  0x00007ffff781d018 in QMainWindow::statusBar() const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#2  0x00000000004ff27a in Brewtarget::log (lt=Brewtarget::LogType_ERROR, message=...) at /home/philip/myc/brewtarget/brewtarget/src/brewtarget.cpp:1040
#3  0x00000000004ff3b3 in Brewtarget::logE (message=...) at /home/philip/myc/brewtarget/brewtarget/src/brewtarget.cpp:1050
#4  0x00000000004bc4f2 in Database::get (this=0xcc92b0, table=Brewtarget::HOPTABLE, key=1, col_name=0x6d0e3f "name")
at /home/philip/myc/brewtarget/brewtarget/src/database.h:126
#5  0x00000000004bbaea in BeerXMLElement::get (this=0xc6d230, col_name=0x6d0e3f "name") at /home/philip/myc/brewtarget/brewtarget/src/BeerXMLElement.cpp:203
#6  0x00000000005b3541 in Hop::name (this=0xc6d230) at /home/philip/myc/brewtarget/brewtarget/src/hop.cpp:287
#7  0x00000000005c08bc in HopTableModel::data (this=0x114e3f0, index=..., role=0) at /home/philip/myc/brewtarget/brewtarget/src/HopTableModel.cpp:261
#8  0x00007ffff30ee948 in QSortFilterProxyModel::data(QModelIndex const&, int) const () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007ffff79a0286 in QStyledItemDelegate::initStyleOption(QStyleOptionViewItem*, QModelIndex const&) const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#10 0x00007ffff799ea2f in QStyledItemDelegate::sizeHint(QStyleOptionViewItem const&, QModelIndex const&) const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#11 0x00007ffff7942811 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007ffff7942aea in QTableView::sizeHintForRow(int) const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#13 0x00007ffff79224a7 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007ffff7922fc9 in QHeaderView::visualIndexAt(int) const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00007ffff79422c9 in QTableView::sizeHintForColumn(int) const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#16 0x00007ffff79224a7 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007ffff7922da9 in QHeaderView::length() const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007ffff7948dc3 in QTableView::updateGeometries() () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#19 0x00007ffff79101e9 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x00007ffff314cfdd in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x00007ffff7928ead in QHeaderView::viewportEvent(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00007ffff311dbba in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#23 0x00007ffff76c610c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#24 0x00007ffff76cb600 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#25 0x00007ffff311ddcb in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007ffff770089d in QWidgetPrivate::hideChildren(bool) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#27 0x00007ffff7700875 in QWidgetPrivate::hideChildren(bool) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#28 0x00007ffff7700875 in QWidgetPrivate::hideChildren(bool) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#29 0x00007ffff7700aab in QWidgetPrivate::hide_helper() () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#30 0x00007ffff77042d0 in QWidget::setVisible(bool) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#31 0x00007ffff78c59ef in QDialog::setVisible(bool) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#32 0x00007ffff78c37c1 in QDialog::~QDialog() () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#33 0x00000000006ba304 in HopDialog::~HopDialog (this=0x111eae0, __in_chrg=<optimized out>) at /home/philip/myc/brewtarget/build/src/../../brewtarget/src/HopDialog.h:51
#34 0x00000000006ba33c in HopDialog::~HopDialog (this=0x111eae0, __in_chrg=<optimized out>) at /home/philip/myc/brewtarget/build/src/../../brewtarget/src/HopDialog.h:51
#35 0x00007ffff314b96c in QObjectPrivate::deleteChildren() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#36 0x00007ffff770131a in QWidget::~QWidget() () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#37 0x00000000006ba67f in MainWindow::~MainWindow (this=0xdef610, __in_chrg=<optimized out>) at /home/philip/myc/brewtarget/build/src/../../brewtarget/src/MainWindow.h:104
#38 0x00000000006ba738 in MainWindow::~MainWindow (this=0xdef610, __in_chrg=<optimized out>) at /home/philip/myc/brewtarget/build/src/../../brewtarget/src/MainWindow.h:104
#39 0x00000000004fa953 in Brewtarget::cleanup () at /home/philip/myc/brewtarget/brewtarget/src/brewtarget.cpp:556
#40 0x00000000004faa66 in Brewtarget::run () at /home/philip/myc/brewtarget/brewtarget/src/brewtarget.cpp:587
#41 0x00000000004b82f9 in main (argc=1, argv=0x7fffffffe0f8) at /home/philip/myc/brewtarget/brewtarget/src/main.cpp:53

@rocketman768
Copy link
Member

Valgrind reports a bunch of invalid reads of size 8 at exit.

@theophae
Copy link
Contributor Author

From my first investigation, it seems that the crash happens because the error log occurs in the same time of the cleanup function. It crashes at the line if( _mainWindow && _mainWindow->statusBar() ) because _mainWindow deletion is not over, but when it tries to do _mainWindow->statusBar(), it crashes. I've tried to add _mainWindow = 0 after delete _mainWindow in the cleanup function, but the first condition is still true.

@rocketman768
Copy link
Member

Ok, so there is a race condition between logging and exiting we should fix.
Why does the database log an error in the first place?
On Thu, Nov 12, 2015 at 2:10 AM theophae [email protected] wrote:

From my first investigation, it seems that the crash happens because the
error log occurs in the same time of the cleanup function. It crashes at
the line if( _mainWindow && _mainWindow->statusBar() ) because
_mainWindow deletion is not over, but when it tries to do
_mainWindow->statusBar(), it crashes. I've tried to add _mainWindow = 0
after delete _mainWindow in the cleanup function, but the first condition
is still true.

—
Reply to this email directly or view it on GitHub
#116 (comment)
.

@rocketman768 rocketman768 modified the milestones: v2.3.0, v2.4.0 Jan 3, 2016
@mikfire
Copy link
Contributor

mikfire commented Jul 10, 2016

This is a snotty bug. As far as I can tell, you don't even need to delete the hop. Just add it and then close brewtarget. Core will be dumped. Deleting an existing hop won't cause a problem.

I can somewhat clean up the core dump, but then I get a mass of "database not open errors" and "
ERROR : Database::get(): SELECT * FROM hop WHERE id=:id (name) Parameter count mismatch" errors, all from the hop table. It is only the hops which cause the problem -- the other elements behave properly.

Oddly enough. In MainWindow::closeEvent(), we call Database::instance().unload(). Control passes back to Brewtarget::run(), which tears the windows down and then calls Database::dropInstance(). If I move the unload() call to dropInstance, all the problems go away.

I suspect the race condition is that we have closed the database before we have torn all the windows down. But I have no idea why the hops alone cause the problem.

I've a simple patch if we are willing to accept "this fixes it, but I don't know why".

@mikfire
Copy link
Contributor

mikfire commented Jul 10, 2016

Hmm.

This fixes it too:

HopDialog::~HopDialog()
{
    hopTableModel->observeDatabase(false);
}

It still doesn't explain why every other bleeding dialog doesn't do this.

@mikfire
Copy link
Contributor

mikfire commented Jul 16, 2016

What's really annoying me is that if you add a hop by pressing the + on the hop tab, this crash doesn't happen. So there's a difference between how the hop tab is opening the editor and how the tree is that I just don't understand

mikfire pushed a commit to mikfire/brewtarget that referenced this issue Jul 27, 2016
Closes Brewtarget#116 - Crash at closing after deleting hop
7bd11d5 
Deleting had nothing to do with it, but adding a new hop did. For reasons I
still do NOT understand, the hop table was attempting to refresh itself on
delete. I went through any number of interations, but could never quite figure
out why.

This is the easiest of the fixes I found -- the database isn't closed until
after we tear all the windows down. Since MainWindow doesn't open the DB, it
shouldn't close it either. I still cannot explain why the hop editor alone
does this, and not the misc editor.
mikfire added a commit that referenced this issue Jul 29, 2016
@mikfire
Merge pull request #300 from mikfire/bugs/deletehopcore
a98b3ed 
Closes #116 - Crash at closing after deleting hop
@mikfire
Copy link
Contributor

mikfire commented Jul 29, 2016

Closed with merge request #300. Not the cleanest solution, but it solves the immediate problem

@mikfire mikfire closed this as completed Jul 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug high priority
Projects
None yet
Milestone
v2.4.0
Development

No branches or pull requests
3 participants
@rocketman768 @mikfire @theophae