Skip to content

Commit

Permalink
Followup to d897aa5; fix crash on Mac
Browse files Browse the repository at this point in the history
  • Loading branch information
@dakcarto
dakcarto committed Nov 21, 2014
1 parent 9ceb5ce commit 9837f43
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/app/qgsbrowserdockwidget.cpp
View file
Expand Up @@ -811,6 +811,9 @@ QStringList QgsBrowserDockWidget::expandedPathsList( const QModelIndex & proxyIn
{
QStringList paths;

if ( !proxyIndex.isValid() )
return paths;

for ( int i = 0; i < mProxyModel->rowCount( proxyIndex ); i++ )
{
QModelIndex childProxyIndex = mProxyModel->index( i, 0, proxyIndex );
Expand Down

4 comments on commit 9837f43

@blazek
Copy link
Member

@blazek blazek commented on 9837f43 Nov 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to revert this in 445675a because the method is intentionally called with invalid proxyIndex (for root). I don't see any problem in this method because it is legal to call rowCount() and index() with invalid index and item pointer is checked.

Was it really crashing in this method? Maybe it was crashing the next time qgis was started and browser tried to expand paths? Can you post backtrace?

@dakcarto
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @blazek,

The app crashes on startup. Here is the related bits of the trace:

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000
...
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   org.qgis.qgis-dev   0x000000010de99a39 QgsBrowserDockWidget::expandedPathsList(QModelIndex const&) + 89 (qgsbrowserdockwidget.cpp:814)
1   org.qgis.qgis-dev   0x000000010de95832 QgsBrowserDockWidget::saveState() + 162 (qabstractitemmodel.h:66)
2   org.qgis.qgis-dev   0x000000010de9574a QgsBrowserDockWidget::hideEvent(QHideEvent*) + 106 (qgsbrowserdockwidget.cpp:320)
3   QtGui               0x00000001122f6c0d QWidget::event(QEvent*) + 1403
4   QtGui               0x00000001125d75f3 QDockWidget::event(QEvent*) + 389
5   QtGui               0x00000001122b8ed5 QApplicationPrivate::notify_helper(QObject*, QEvent*) + 195
6   QtGui               0x00000001122bba02 QApplication::notify(QObject*, QEvent*) + 6498
7   org.qgis.qgis2_core 0x0000000110fbefb0 QgsApplication::notify(QObject*, QEvent*) + 96 (qgsapplication.cpp:252)
8   QtCore              0x000000011201fbb8 QCoreApplication::notifyInternal(QObject*, QEvent*) + 118
9   QtGui               0x00000001122f5dfd QWidgetPrivate::hide_helper() + 249
10  QtGui               0x00000001122f61dc QWidget::setVisible(bool) + 282
11  org.qgis.qgis-dev   0x000000010ddf1150 QgisApp::QgisApp(QSplashScreen*, bool, QWidget*, QFlags<Qt::WindowType>) + 3200 (qgisapp.cpp:616)
12  org.qgis.qgis-dev   0x000000010ddeb292 main + 17506 (main.cpp:885)
13  libdyld.dylib       0x00007fff8f7ef5fd start + 1

I substituted the following (to check if the index's internal pointer is not 0 instead) and it avoids the startup crash:

  if ( !proxyIndex.internalPointer() )
    return paths;

Does that work out OK for your implementation, instead?

@blazek
Copy link
Member

@blazek blazek commented on 9837f43 Dec 1, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internalPointer() won't help because invalid index has null internalPointer().

It seems however that hideEvent() is called before showEvent() on Mac. mModel and mProxyModel are created in showEvent(). I added checks in 4fde34a.

@dakcarto
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blazek I can confirm here that 4fde34a does fix the startup crash. Thanks!

Please sign in to comment.