yeesian / ArchGDAL.jl

A high level API for GDAL - Geospatial Data Abstraction Library
https://yeesian.github.io/ArchGDAL.jl/stable/
Other
141 stars 27 forks source link

fix segfaults on nightly #348

Closed visr closed 1 year ago

visr commented 1 year ago

It seems that the featuredefinition was finalized too soon. This seems to be fine, also works if I put a GC in between. Edit: no, see below, basically we need f(obj.ptr) to GC.@preserve obj f(obj.ptr) everywhere.

Partially fixes #347, let's close that and remove ArchGDAL from the PkgEval exceptions when all tests are working again in nightly.

visr commented 1 year ago

Test are still failing, but I don't see segfaults on nightly at least. Most failing tests are due to new GDAL, #345, which can be fixed in a separate PR.

For the error "type Stateful has no field taken" is just created https://github.com/meggart/DiskArrays.jl/issues/80.

I should say that I don't fully understand why this PR works, and that looking for layerdefn in the test dir, I find more examples of the old pattern, not sure why they don't segfault.

visr commented 1 year ago

I was a bit to quick with this PR, there were actually many more segfaults and other errors triggered in the tests, all related to what seems a more aggresive GC.

In LibGEOS.jl similar GC preserve macros have also been added. I should note that I changed all the ones that caused test errors, and then a bunch that did not cause test errors, but had the same issue. I did not yet go over all files systematically to look for them all.

visr commented 1 year ago

Might be better to follow the approach of https://github.com/yeesian/ArchGDAL.jl/issues/347#issuecomment-1327617835

visr commented 1 year ago

Yeah let's close this, I'm convinced that that approach is better and also works.