Skip to content

Fix segfault when computing filter_geom() cubes - #1

Merged
NewGraphEnvironment merged 2 commits into
newgraphfrom
110-segfault-in-filter-geom-on-compute
Sep 1, 2026
Merged

NewGraphEnvironment merged 2 commits into
newgraphfrom
110-segfault-in-filter-geom-on-compute

Conversation

@NewGraphEnvironment

Copy link
Copy Markdown
Owner

Summary

  • Fix the segfault that crashed every computed filter_geom() cube (upstream Segfault in filter_geom() on compute (0.7.4, macOS arm64) appelmar/gdalcubes#110): read_chunk() handed a stack-allocated OGRSpatialReference to OGRPolygon::assignSpatialReference(), which takes refcounted ownership and Release()s it in ~OGRGeometry() — a use-after-destruction under GDAL 3's pimpl'd SRS. The SRS assignment is dropped; Contains()/Intersects() never use it and both geometries are already in the cube CRS.
  • Close out the remaining log-and-continue error paths in filter_geom.cpp with proper throws: GetDriverByName("GPKG"), GetNextFeature(), GDALRasterize() (was a latent second null-deref segfault), and RasterIO() (was silently reading an uninitialized mask buffer).
  • Add inst/tinytest/test_filter_geom.R — filter_geom() previously had zero test coverage — asserting exact burned-cell count, inside/outside values, and chunk-layout invariance (exercises the chunk-within-polygon copy fast path).

Related Issues

Root cause evidence

  • Reproduced on this branch pre-fix: worker SIGSEGV "invalid permissions" at fault address 0x138 (GDAL 3.13.3) vs the issue's 0x120 (GDAL 3.8.5) — the address is offsetof(OGRSpatialReference::Private, nRefCount), which shifts with GDAL's private struct layout. The assignSpatialReference call was the only one in the codebase, which is why only filter_geom() crashed.
  • Post-fix: repro writes a correctly masked netCDF (10,000 interior cells = 1, exterior all NA; identical mask under 50×50 chunking).

Test plan

  • Segfault repro from the issue passes (no crash, correct masked output)
  • Full tinytest suite green locally (125 results, all OK) on macOS arm64 / R 4.5.2 / GDAL 3.13.3
  • /code-check: two review rounds, both clean

Notes

  • Base is newgraph (org base branch), so the diff contains only the fix, NEWS entry, and test — suitable for porting upstream as-is.
  • master was fast-forwarded to upstream v0.7.4 RC (2cc0d46) first; the 0.7.2 source no longer compiled against GDAL 3.13 and the issue is filed against 0.7.4.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ENJ9rVrvd9BrixVKtgWtPu

NewGraphEnvironment and others added 2 commits August 31, 2026 18:27
filter_geom_cube::read_chunk() assigned a stack-allocated
OGRSpatialReference to the chunk polygon via assignSpatialReference(),
which takes refcounted ownership: ~OGRGeometry() calls Release() on the
SRS, and under GDAL 3 (pimpl'd OGRSpatialReference) the stack object is
destroyed before the polygon at every scope exit, so the release writes
through a dangling pointer. Every computed filter_geom() cube crashed
the worker process with SIGSEGV ("invalid permissions", fault address =
offsetof of the refcount inside GDAL's private SRS struct: 0x120 on GDAL
3.8.5, 0x138 on 3.13.3). Drop the assignment: Contains()/Intersects()
do not use the SRS, and both geometries are already in the cube CRS.

Also close out the log-and-continue error paths in the same code:

- GetDriverByName("GPKG") null check before Create() (constructor)
- GetNextFeature() null check before dereferencing the feature
- GDALRasterize() failure now throws instead of falling through to a
  null GetRasterBand() dereference
- RasterIO() failure now throws instead of reading an uninitialized
  mask buffer

Fixes appelmar#110

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENJ9rVrvd9BrixVKtgWtPu
Covers the segfault fixed in the previous commit
(appelmar#110): computes a
filter_geom() cube from a dummy cube via as_array(), asserting the
exact burned-cell count, inside/outside values, and that the mask is
identical under 50x50 chunking (exercising the chunk-within-polygon
copy fast path). filter_geom() previously had no tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENJ9rVrvd9BrixVKtgWtPu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant