Bug report #13679

QGIS master crashes when closing DB Manager in preview mode for PostGIS layers if password is not saved

Added by salvatore fiandaca over 4 years ago. Updated over 4 years ago.

Status:Closed
Priority:Severe/Regression
Assignee:Sandro Santilli
Category:DB Manager
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:fixed/implemented
Crashes QGIS or corrupts data:Yes Copied to github as #:21711

Description

New description:

Create a PostGIS connection and do not save the password.

Enter DB Manager, enter the password for the connection, select a layer and open the "preview" tab

Close db manager

Crash

This happens on QGIS master (tested on Windows), while on the latest 2.12 and 2.8 is ok

Old Description:

http://1drv.ms/1hPEdHP

plugin.jpg (246 KB) salvatore fiandaca, 2015-11-06 07:45 AM

Associated revisions

Revision 47013f7b
Added by Salvatore Larosa over 4 years ago

[dbmanager] Fixes #13679 - QGIS master crashes when closing DB Manager in preview mode

History

#1 Updated by Paul Kanelli over 4 years ago

QGIS 2.12 causes a crash when closing the 'DB Manager' in the 'Preview' view (Win7).

#2 Updated by salvatore fiandaca over 4 years ago

QGIS 2.12 causes a minidump when closing the 'DB Manager' in the 'Preview' view with vector postgis (Win10) (see Attachment)

#3 Updated by salvatore fiandaca over 4 years ago

  • Assignee deleted (Jürgen Fischer)

ma nessun altro ha questo problema??? no one else has this problem ???

#4 Updated by Steven Mizuno over 4 years ago

  • Category changed from Data Provider/PostGIS to Python plugins

It appears to me that the window is closing before the preview object is deleted. The variable that holds the preview is set to None, the same as when a different database item is chosen. I found that that choosing a different item, but not previewing it, and then closing DB Manager doesn't cause a crash.

Also, this is not specific to PostGIS -- I tried with a SpatiaLite table as well, so I changed the Category to better reflect where the problem occurs.

#5 Updated by Giovanni Manghi over 4 years ago

  • Category changed from Python plugins to DB Manager
  • Status changed from Open to Feedback

Cannot replicate the issue on master/windows following the same steps, please update, try again and report back, thanks!

#6 Updated by salvatore fiandaca over 4 years ago

QGIS 2.12 causes a minidump when closing the 'DB Manager' in the 'Preview' view with vector postgis (Win10) (see Attachment)

http://1drv.ms/1XUuSyS

#7 Updated by Giovanni Manghi over 4 years ago

salvatore fiandaca wrote:

QGIS 2.12 causes a minidump when closing the 'DB Manager' in the 'Preview' view with vector postgis (Win10) (see Attachment)

http://1drv.ms/1XUuSyS

I just tried on the latest master (and 2.12 too) and does not happen, could you try it too (and remove/disable all the plugins too)?

#8 Updated by salvatore fiandaca over 4 years ago

I started with --no plugins; I can not find DBManager

#9 Updated by Giovanni Manghi over 4 years ago

salvatore fiandaca wrote:

I started with --no plugins; I can not find DBManager

sorry, I meant 3rd party plugins.

#10 Updated by salvatore fiandaca over 4 years ago

remove all 3rd party plugins, the mini-dump remains.!!!!

#11 Updated by Giovanni Manghi over 4 years ago

salvatore fiandaca wrote:

remove all 3rd party plugins, the mini-dump remains.!!!!

can you replicate on other machines?

#12 Updated by salvatore fiandaca over 4 years ago

I installed QGIS 2.12 in another machine (6 GB RAM, 64-bit, Win 10, like mine), the mini-dump remains.

#13 Updated by Giovanni Manghi over 4 years ago

  • Target version changed from Version 2.12 to Future Release - High Priority
  • Affected QGIS version changed from master to 2.12.0

salvatore fiandaca wrote:

I installed QGIS 2.12 in another machine (6 GB RAM, 64-bit, Win 10, like mine), the mini-dump remains.

Well of course it seems (but I can e wrong) that some local factor is causing the issue. Could you please try the dev/master version and try again? Thanks.

#14 Updated by Steven Mizuno over 4 years ago

It occurred to me that preventing the mapcanvas from rendering might help.
have found that in db_manager.py, def closeEvent(self,e) using:

    self.unregisterAllActions()
    self.preview.freeze(True)       #<-- Added line: preview (LayerPreview) is derived from QgsMapCanvas
    self.preview.loadPreview(None)

seems to prevent the crash. This was tested on the 32-bit build of master rev.93bd825

Further testing with rev. 93bd825 the crash occurs on 32-bit Windows builds, but doesn't happen as often (for a while, not on 10 or more tries) on 64-bit; adding the freeze() line stops the crash on both.

Might there a problem with passing references across threads somewhere in the map rendering similar to the Browser crashing (see #13738)?

Using setRenderFlag(False) instead of freeze() does not prevent crash -- why?

Further investigation of QgsMapCanvas -- freeze() and setRenderFlag() -- Both do the same thing, which is to allow/disallow rendering the map. However, the setRenderFlag() function does something in addition. The mRenderFlag is tested and if false calls stopRendering() [and refresh() if true -- not relevant to this problem]

So using freeze(True) is a better choice.

#15 Updated by Giovanni Manghi over 4 years ago

  • Target version deleted (Future Release - High Priority)

salvatore fiandaca wrote:

I installed QGIS 2.12 in another machine (6 GB RAM, 64-bit, Win 10, like mine), the mini-dump remains.

do you still have issue? I cannot really replicate this one in my computer and any other one. I have just gave a postgis training to a classroom full of people (all Windows) and no one had this issue.

#16 Updated by Giovanni Manghi over 4 years ago

Steven Mizuno wrote:

It occurred to me that preventing the mapcanvas from rendering might help.
have found that in db_manager.py, def closeEvent(self,e) using:

does this fixes this issue that I can't confirm? if yes then if possible make a patch and pull request. Thanks!

#17 Updated by salvatore fiandaca over 4 years ago

Giovanni Manghi wrote:

salvatore fiandaca wrote:

I installed QGIS 2.12 in another machine (6 GB RAM, 64-bit, Win 10, like mine), the mini-dump remains.

do you still have issue? I cannot really replicate this one in my computer and any other one. I have just gave a postgis training to a classroom full of people (all Windows) and no one had this issue.

yes, I always have the problem. Patience.

#18 Updated by Giovanni Manghi over 4 years ago

  • Priority changed from High to Severe/Regression
  • Affected QGIS version changed from 2.12.0 to master
  • Status changed from Feedback to Open
  • Subject changed from minidumps exit from DBManager to QGIS master crashes when closing DB Manager in preview mode for PostGIS layers if password is not saved

#19 Updated by Sandro Santilli over 4 years ago

I cannot reproduce this on maste (a0e6d49) with ubuntu-14.04 -- any chance you can re-test this Salvatore ?

#20 Updated by Sandro Santilli over 4 years ago

  • Status changed from Open to In Progress
  • Assignee set to Sandro Santilli

Oh wait, I take it back. I was missing the "close" step.
It's confirmed here !

#21 Updated by Sandro Santilli over 4 years ago

RuntimeError: wrapped C/C++ object of type QgsVectorLayer has been deleted 
Traceback (most recent call last):
  File "/usr/src/qgis/build/master/output/python/plugins/db_manager/db_manager.py", line 62, in closeEvent
    self.preview.loadPreview(None)
  File "/usr/src/qgis/build/master/output/python/plugins/db_manager/layer_preview.py", line 57, in loadPreview
    self._clear()
  File "/usr/src/qgis/build/master/output/python/plugins/db_manager/layer_preview.py", line 85, in _clear
    self._loadTablePreview(None)
  File "/usr/src/qgis/build/master/output/python/plugins/db_manager/layer_preview.py", line 118, in _loadTablePreview
    QgsMapLayerRegistry.instance().removeMapLayers([self.currentLayer.id()])
RuntimeError: wrapped C/C++ object of type QgsVectorLayer has been deleted

#22 Updated by salvatore fiandaca over 4 years ago

Sandro,
il minidump mi perseguita, sia se utilizzo db postgis che spatialite.
poi ho riscontrato, (sempre su dbmanager) altri problemi legati a file csv; cioè se carico in qgis un file csv con CTRL+SHIFT+V e poi cerco di importarlo in un db spatialite tramite dbmanager mi da errore ' errore 5 errore nella creazione dei campi'.

ciao

#23 Updated by Sandro Santilli over 4 years ago

The call to loadPreview on close() was added by Giuseppe to fix #12938
Giuseppe, any idea about my backtrace ?

Salvatore please use different tickets for different bugs. The fact they all result in a "minidump" do not make them be the same bug.

#24 Updated by Sandro Santilli over 4 years ago

I cannot always reproduce it, btw

#25 Updated by Sandro Santilli over 4 years ago

Actually, I cannot reproduce it anymore :/

#26 Updated by salvatore fiandaca over 4 years ago

il minidump si verifica solo ed esclusivamente se facciamo 'anteprima' del layer (Postgis e spatialite);
infatti evito di fare l'anteprima per lavorare. (QGIS 2.12.3)

#27 Updated by salvatore fiandaca over 4 years ago

the mini-dump occurs if and only if I 'preview' layer (Postgis and spatialite);
In fact, it avoids doing the preview to work. (QGIS 2.12.3)

#28 Updated by Giuseppe Sucameli over 4 years ago

No crash here (Ubuntu 14.04 64bit), but in log panel I can see the following messages:

2016-01-18T19:58:26 1 QObject::connect: Cannot connect (null)::repaintRequested() to LayerPreview::refresh()
2016-01-18T19:58:26 1 QObject::connect: Cannot connect (null)::layerCrsChanged() to LayerPreview::layerCrsChange()
2016-01-18T19:58:28 1 Loading a style file that was saved with an older version of qgis (saved in 2.11.0-Master, loaded in 2.13.0-Master). Problems may occur.
2016-01-18T19:58:28 1 QObject::connect: Cannot connect (null)::repaintRequested() to LayerPreview::refresh()
2016-01-18T19:58:28 1 QObject::connect: Cannot connect (null)::layerCrsChanged() to LayerPreview::layerCrsChange()

that is similar to the issue reported by Sandro (strk): the layer was deleted.

In my case it seems due to a canvas refresh (repaintRequested), so I guess it's a race-condition due to multithread (but I've never looked at the current implementation, so I cannot say anything more).

Surely the issue can occurs if the layer is destroyed using delete (and not deleteLater() which ensure signal disconnection).

Since objects in python are destroyed when the last ref variable goes out of scope (not by a deferred deletion event), something wrong can happen.

#29 Updated by Sandro Santilli over 4 years ago

Is there any way to make the python wrapper call deleteLater rather than delete, on dropping last reference ?

#30 Updated by Giuseppe Sucameli over 4 years ago

In the meantime, freezing preview canvas and/or change the selected tab (hiding preview) on close could mitigate the issue since the canvas is not repainted, but I haven't tested it.

#31 Updated by Sandro Santilli over 4 years ago

I still cannot reproduce with b2a1273.
Could it have been done to the recent threading fixes ?
Which version did you try with, Giuseppe ?
Also, does having or not to enter a password make a difference ?

#32 Updated by Sandro Santilli over 4 years ago

  • Status changed from In Progress to Feedback

#33 Updated by Steven Mizuno over 4 years ago

I have tried rev. 81448f8 (2016-01-19) and DB Manager closing still crashes QGIS if a layer is previewed. This is using 32-bit Windows build on Win8 or Win7 64-bit systems.

I do see the following messages when the Preview tab is clicked for a layer. These occur when the layer is shown, not when DB Manager is closed, so I don't know if they are relevant.

2016-01-20T19:49:13    1    QObject::connect: Cannot connect (null)::repaintRequested() to LayerPreview::refresh()
2016-01-20T19:49:13    1    QObject::connect: Cannot connect (null)::layerCrsChanged() to LayerPreview::layerCrsChange()

As I previously reported this occurs on SpatiaLite layers as well as PostGIS, so I believe that a password is NOT a factor (see comment 31 and the subject).

From my notes about the problem I found that I captured a call stack trace using WinDbg -- this was from an earlier rev., however. The top line is the last call logged before the crash.

QtGui4!QGraphicsItem::prepareGeometryChange+0x9
qgis_gui!QgsMapCanvasMap::setContent(class QImage * image = 0x004ec4c0, class QgsRectangle * rect = 0x004ec38c)+0x13d
qgis_gui!QgsMapCanvas::rendererJobFinished(void)+0xbd5
qgis_gui!QgsMapCanvas::qt_static_metacall(class QObject * _o = 0x042ac468, QMetaObject::Call _c = InvokeMetaMethod (0n0), int _id = 0n40, void ** _a = 0x004ec5a4)+0x3f0
QtCore4!QMetaObject::activate+0x3c6
qgis_core!QgsMapRendererJob::finished(void)+0x1a
qgis_core!QgsMapRendererSequentialJob::internalFinished(void)+0x15b
qgis_core!QgsMapRendererSequentialJob::qt_static_metacall(class QObject * _o = 0x043ff0f8, QMetaObject::Call _c = InvokeMetaMethod (0n0), int _id = 0n0, void ** _a = 0x004ec684)+0x2d
QtCore4!QMetaObject::activate+0x3c6
qgis_core!QgsMapRendererJob::finished(void)+0x1a
qgis_core!QgsMapRendererCustomPainterJob::futureFinished(void)+0xaf
qgis_core!QgsMapRendererCustomPainterJob::cancel(void)+0x22a
qgis_core!QgsMapRendererSequentialJob::cancel(void)+0x9f
qgis_gui!QgsMapCanvas::~QgsMapCanvas(void)+0x16cqgis_gui!QgsMapCanvas::~QgsMapCanvas(void)+0x16c
_gui!sipQgsMapCanvas::~sipQgsMapCanvas(void)+0x6b
_gui!sipQgsMapCanvas::`scalar deleting destructor'(void)+0xf
QtCore4!QObjectPrivate::deleteChildren+0x72
QtGui4!QWidget::~QWidget+0x5c3
QtGui4!QWidgetPrivate::invalidateBuffer_resizeHelper+0x8af
QtGui4!QRegion::~QRegion+0xb
QtGui4!QStackedWidget::~QStackedWidget+0x7a

#34 Updated by Sandro Santilli over 4 years ago

Does 09ce8004fd2ca9e911b84dd4de59bafb47037b44 (from Salvatore Larosa) fix the crash ?
Giuseppe: wouldn't it be better to avoid calling a "refresh" function on "close" ? It does sound like an hack...

#35 Updated by salvatore fiandaca over 4 years ago

Salvatore Larosa has solved the mini-dump. Hooray!

#36 Updated by Sandro Santilli over 4 years ago

  • Resolution set to fixed/implemented
  • Status changed from Feedback to Closed

And Salvatore Fianca found the bug and tested the fix. A name, a plan!

Also available in: Atom PDF