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.

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

Editing a GeoPackage feature with spaces in the field names can crash QGIS #29630

Closed
qgib opened this issue Apr 9, 2019 · 8 comments · Fixed by #29997
Closed

Editing a GeoPackage feature with spaces in the field names can crash QGIS #29630

qgib opened this issue Apr 9, 2019 · 8 comments · Fixed by #29997
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Digitizing Related to feature digitizing map tools or functionality High Priority

Comments

@qgib
Copy link
Contributor

qgib commented Apr 9, 2019

Author Name: Matt Yoder (@yomatters)
Original Redmine Issue: 21815
Affected QGIS version: 3.6.1
Redmine category:digitising


Steps to reproduce:

  1. Add the attached GeoPackage layer to the map.
  2. Toggle editing.
  3. Use the Vertex Tool to move a point.

Renaming the source fields to remove spaces from the field names before editing seems to prevent the crash.

Crash output:

Crash ID: 1bd9d85e72ea45ae97a3eff7dea07ce46a32dcc6


Stack Trace


Index::~Index :
qsort :
Index::~Index :
Index::~Index :
Index::~Index :
Index::~Index :
Index::~Index :
Index::~Index :
Index::~Index :
Index::~Index :
Index::~Index :
QgsPointLocator::onFeatureDeleted :
QgsPointLocator::onGeometryChanged :
QMetaObject::activate :
QgsVectorLayer::geometryChanged :
QMetaObject::activate :
QgsVectorLayerEditBuffer::geometryChanged :
QUndoStack::push :
QgsVectorLayerEditBuffer::changeGeometry :
QgsVectorLayer::changeGeometry :
QgsVertexTool::applyEditsToLayers :
QgsVertexTool::moveVertex :
QgsVertexTool::cadCanvasReleaseEvent :
QgsMapToolAdvancedDigitizing::canvasReleaseEvent :
QgsMapCanvas::mouseReleaseEvent :
QWidget::event :
QFrame::event :
QGraphicsView::viewportEvent :
QCoreApplicationPrivate::sendThroughObjectEventFilters :
QApplicationPrivate::notify_helper :
QApplication::notify :
QgsApplication::notify :
QCoreApplication::notifyInternal2 :
QApplicationPrivate::sendMouseEvent :
QSizePolicy::QSizePolicy :
QSizePolicy::QSizePolicy :
QApplicationPrivate::notify_helper :
QApplication::notify :
QgsApplication::notify :
QCoreApplication::notifyInternal2 :
QGuiApplicationPrivate::processMouseEvent :
QWindowSystemInterface::sendWindowSystemEvents :
QEventDispatcherWin32::processEvents :
TranslateMessageEx :
TranslateMessage :
QEventDispatcherWin32::processEvents :
qt_plugin_query_metadata :
QEventLoop::exec :
QCoreApplication::exec :
main :
BaseThreadInitThunk :
RtlUserThreadStart :




QGIS Info
QGIS Version: 3.6.1-Noosa
QGIS code revision: 2468226bc9
Compiled against Qt: 5.11.2
Running against Qt: 5.11.2
Compiled against GDAL: 2.4.1
Running against GDAL: 2.4.1



System Info
CPU Type: x86_64
Kernel Type: winnt
Kernel Version: 6.1.7601


@qgib
Copy link
Contributor Author

qgib commented Apr 9, 2019

Author Name: Brett Carlock (@Saijin-Naib)


Can confirm here with the supplied data.

@qgib
Copy link
Contributor Author

qgib commented Apr 10, 2019

Author Name: Giovanni Manghi (@gioman)


No crash here on 3.6.1 on linux.

Please also try a new/clean (qgis) profile, no 3rd party plugins installed.


  • priority_id was changed from Normal to High
  • status_id was changed from Open to Feedback
  • category_id was changed from Editing to Digitising

@qgib
Copy link
Contributor Author

qgib commented Apr 10, 2019

Author Name: Matt Yoder (@yomatters)


I can reproduce the crash with a clean profile on both Windows and Mac OS. However, I noticed in my testing that not all of the points in the layer cause the crash when they are moved. The point with fid 97 consistently causes the crash for me when I try to move it using the Vertex Tool.

@qgib
Copy link
Contributor Author

qgib commented Apr 10, 2019

Author Name: Giovanni Manghi (@gioman)


Matt Yoder wrote:

I can reproduce the crash with a clean profile on both Windows and Mac OS. However, I noticed in my testing that not all of the points in the layer cause the crash when they are moved. The point with fid 97 consistently causes the crash for me when I try to move it using the Vertex Tool.

not on linux.
Have you tried with a new/clean profile?

@qgib
Copy link
Contributor Author

qgib commented Apr 10, 2019

Author Name: Matt Yoder (@yomatters)


Yes, I went to Settings -> New Profile to create a new profile. I get the same crash under Windows and Mac OS with the clean profile. I just tested on Ubuntu as well, but I don't get the crash there.

@qgib
Copy link
Contributor Author

qgib commented Apr 10, 2019

Author Name: Giovanni Manghi (@gioman)


  • status_id was changed from Feedback to Open

@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority Digitizing Related to feature digitizing map tools or functionality Crash/Data Corruption labels May 25, 2019
@PeterPetrik PeterPetrik self-assigned this May 28, 2019
@PeterPetrik
Copy link
Contributor

PeterPetrik commented May 28, 2019

MacOS build uses 1.9.0, Windows uses 1.9.0-1. Linux has 1.8.5. The bug is not present in 1.8.5 version and hence it cannot be reproduced in linux.

the crash does not happen when you first move some other vertex and just after vertex 97. This suggests that it is something with the RTree internals.

The whitespace in source fields should not have any affect on this, since RTree does not use fields, just fids and geometries.

duplicate od #28751

@PeterPetrik PeterPetrik removed their assignment May 28, 2019
@wonder-sk
Copy link
Member

wonder-sk commented May 28, 2019

Tested on Linux and I can't replicate with libspatialindex master (nearly the same as 1.9.0).

BUT: looking into libspatialindex bug libspatialindex/libspatialindex#107 and code, it seems that the Node::rstarSplit() code could fail in case of some weird coordinates (nan/inf/?), probably if a whole node is full of them. And indeed, the test layer has a bunch of Point(nan nan) geometries as can be confirmed:

for f in iface.activeLayer().getFeatures(): print(math.isnan(f.geometry().get().x()))

Libspatialindex probably should not allow insertion of features with bounding boxes being NaN, but at the same time we should not feed such features to libspatialindex either - they would not match any spatial search. So I would suggest to skip indexing features in QgsPointLocator::rebuildIndex() where the bounding box is NaN or Inf (using QgsRectangle::isFinite())

Not sure what exactly introduced this change since 1.8.5. My guess is that it was libspatialindex/libspatialindex#61 which reverses how NaN region would be treated by touchesRegion()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Digitizing Related to feature digitizing map tools or functionality High Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants