[QGIS-Developer] Drag and drop to PostGIS crash

[Nyall Dawson]

On 2 June 2017 at 03:31, Radim Blazek <radim.blazek at gmail.com> wrote:
> On Tue, May 30, 2017 at 11:39 PM, Nyall Dawson <nyall.dawson at gmail.com> wrote:
>>> DD works within the browser now but not for layer with measures.
>> I guess this has never been implemented. Looking at ...
>
> I added Z, M and ZM types to postgres provider output and allowed
> AddFeatures for layers with measures (user should be warn on
> application level).

Thanks!

>>> DD from layer tree to postgres/browser imports the layer (without measures) but:
>>> - the layer is removed from layer tree - intended?
>>
>> Definitely not - I hit this too a week or so ago, and it's far from
>> ideal! QgsDockBrowserTreeView::dragEnterEvent/dragMoveEvent/dropEvent
>> has code which is supposed to force a copy action when dragging from
>> the layer tree, but something's broken there. Any ideas Martin?
>
> I found that it makes copy (showing + icon) if dragged with Ctrl. No
> idea why setDropAction( Qt::CopyAction ) in QgsDockBrowserTreeView is
> ignored.
>
> I have no clear opinion if default action should be copy or move in this case.

I think copy should definitely be the ONLY behavior here. Otherwise
there's data loss - user's styling and layer settings will be lost,
and dragging memory layers will lose all features (admittedly dragging
memory layers doesn't copy any features anyway, but that makes it
worse... the contents are irretrievably lost).

I can't work this one out. I think
QgsDockBrowserTreeView::dragEnterEvent() should be calling
QTreeView::dragEnterEvent( e ) - without that it's not drawing the
drop indicators correctly. And in
QgsDockBrowserTreeView::dragMoveEvent if I reset the drop action to
CopyAction both before *and* after calling QTreeView::dragMoveEvent( e
) then the visual appearance of the drag appears correct, and seems
ready to perform a copy action... but the actual action performed is
still a move action.

There's something very screwy here... as far as I can tell we don't
anywhere state that we accept move actions here, and my understanding
is that the default Qt implementations only accept copy actions.

Any ideas?

>
>>> One traceback below, it crashed when DD failed. I think that the
>>> problem is that it is trying to show an error message which can only
>>> be done on main thread?
>>> The code which is run from QgsTaskManager should never interact with GUI?
>>
>> That shouldn't be the case (unless there's a bug somewhere). It's true
>> that tasks should never interact with the GUI in their run()
>> implementation, but QgsTask::finished should only ever be called from
>> the main thread and it should be safe to do GUI operations there. From
>> your crash below it seems like it's crashing after
>> QgsVectorLayerExporterTask::errorOccurred is emitted - but that's only
>> ever done from QgsVectorLayerExporterTask::finished . Could you try
>> putting a temporary assert in QgsVectorLayerExporterTask::finished to
>> ensure that it's working as designed and being called from the main
>> thread?
>
> You are right, errorOccurred is emitted on the main thread.
> Unfortunately I cannot reproduce the crash. It may have something to
> do with how the items in the browser were created, if all pushed to
> main thread or deleted to early instead of deleteLater().

Hmm... I don't like unpronounceable random crashes. Thanks for your
testing though... please let me know if this continues to be an issue!

Nyall