Fixed #32600 -- Fixed Geometry collections and Polygon segmentation fault on macOS ARM64.#15214
Conversation
|
Hello @bpartridge! Thank you for your contribution 💪 As it's your first contribution be sure to check out the patch review checklist. If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket! If you have any design or process questions then you can ask in the Django forum. Welcome aboard ⛵️! |
|
Awesome! Just a small linting about imports to fix. Thanks a lot. |
|
Upon further reviewing this, does this apply as a crashing bug per https://docs.djangoproject.com/en/dev/internals/release-process/#supported-versions-policy ? If so, should I add a release note in preparation for backporting? Do maintainers handle the backporting process, or should I create separate backport PRs? |
felixxm
left a comment
There was a problem hiding this comment.
@bpartridge Thanks for this patch 👍
If so, should I add a release note in preparation for backporting?
Yes, please add a release note to the 4.0.1.txt.
Do maintainers handle the backporting process, or should I create separate backport PRs?
Mergers handle backporting, so you don't need to create separate patches.
be3298a to
44e481d
Compare
|
@bpartridge Thanks for updates 👍 Welcome aboard ⛵ I pushed edits to a release note. |
44e481d to
e768fc0
Compare
08b286d to
b233651
Compare
|
It works perfectly! > pip install -r requirements.txt
Obtaining Django from git+https://github.com/bpartridge/django.git@ticket_32600#egg=Django (from -r requirements.txt (line 3))
Updating ./venv/src/django clone (to revision ticket_32600)
Running command git fetch -q --tags
Running command git reset --hard -q b2336516a8ac6ae754a254a8c1d872da0ca96dc2
Installing build dependencies ... done
Checking if build backend supports build_editable ... done
Getting requirements to build wheel ... done
Preparing metadata (pyproject.toml) ... done
...> python manage.py shell
Python 3.9.9 (main, Nov 21 2021, 03:16:13)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.30.1 -- An enhanced Interactive Python. Type '?' for help.
In [1]: from django.contrib.gis.geos import MultiPolygon
In [2]: p = MultiPolygon()
In [3]: p
Out[3]: <MultiPolygon object at 0x150116088> |
|
@HorebParraud Thanks 👍 |
|
Works for me too! \ö/ |
…ault on macOS ARM64.
b233651 to
19fb838
Compare
|
@aapris Thanks 👍 |
In an attempt to fix the issue reported in #1449 and perhaps solved in django/django#15214.
* Replace c_void_p with c_geom_p In an attempt to fix the issue reported in #1449 and perhaps solved in django/django#15214. * Enable setuptools legacy editable install
https://code.djangoproject.com/ticket/32600
In GIS, when constructing Polygon, MultiPolygon, MultiLineString, or MultiPoint in a process running natively on Apple's M1 processor (arm64 architecture), the GEOS call reliably causes a segmentation fault; this occurs in production applications with various versions of GEOS and Django, as well as when running Django runtests with the latest supported versions of each.
I can confirm that this is indeed a Django bug, and I was able to replicate segfaults on Django's main branch, so it appears the GEOS 3.9 update in https://code.djangoproject.com/ticket/32544 did not fix this. It's also been replicated in e.g. libgeos/geos#528.
It appears that in the current GEOS integration, argument signatures were omitted in favor of automatically casting arguments for a few polygon-related API calls. While this appears to have worked fine on x86 and x64, on macOS arm64 (for native M1 processors), it causes seemingly random data to be provided in place of arguments after the first, reliably causing segmentation faults whenever a Polygon, MultiPolygon, or MultiLineString is constructed. Underlying CFFI internals may be less lenient to unspecified arguments in Apple's implementation than on other platforms. Moving to explicit signatures works fine, and I see no reason having explicit signatures should cause problems on other platforms (though we should ensure CI passes).
All GIS tests now pass, and the changes are fully covered by test_geos. The deeper issue in testing is that CI does not run on Apple hardware, so it will be difficult to prevent regressions. However, the code changes are small and well-documented, and I'd suggest that Apple/arm64 CI will be important but not urgent.
For organizations using older versions of Django, I've also made a monkey-patch available at https://gist.github.com/bpartridge/26a11b28415d706bfb9993fc28767d68 which can be added in any initialization code (or even called from settings.py) before Polygon objects are created. When applied in test settings against the main branch, all GIS tests pass as well; functionality is identical to the PR above.