Skip to content

Fix potential segfault errors#64

Merged
mindflayer merged 11 commits into
mainfrom
segfault-tackling
Jun 8, 2026
Merged

Fix potential segfault errors#64
mindflayer merged 11 commits into
mainfrom
segfault-tackling

Conversation

@mindflayer

@mindflayer mindflayer commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes several C-level memory safety issues in ToGo's Cython bindings that could surface as segfaults under planner-style workloads with heavy geometry extraction, GEOS operations, and garbage collection pressure.

What changed

  • Clone borrowed TG pointers before exposing them as Python-owned wrappers:
    • Geometry.line()
    • Geometry.poly()
    • Poly.exterior
    • Poly.hole(...)
    • internal Ring.from_ptr(...) / Poly._from_c_poly(...) paths
  • Add defensive NULL pointer checks before wrapping TG pointers.
  • Initialize Geometry.geom explicitly in Geometry.__cinit__.
  • Check clone failures and raise Python exceptions instead of creating invalid wrapper objects.
  • Make C buffer allocation paths exception-safe with try/finally.
  • Add checked Python-to-C count conversion before passing sequence lengths into TG APIs.
  • Improve cleanup in Geometry.unary_union(...) by destroying GEOS geometries on all success/error paths.
  • Handle empty meters-grid conversions in the ToGo wrapper layer by cloning empty geometries instead of calling into TGX with empty component arrays.
  • Add regression tests for:
    • wrapper objects surviving parent Geometry / Poly garbage collection
    • empty geometry meters-grid round trips

Why

The previous implementation returned Python wrapper objects around borrowed C pointers owned by parent geometries. If the parent object was garbage-collected while the child wrapper remained in use, later calls could dereference freed memory and crash.

This was especially likely in long-running planner integrations that repeatedly extract rings, polygons, boundaries, or line strings from temporary geometries.

Validation

  • Built successfully with:

    make build-wheel

Copilot AI review requested due to automatic review settings June 8, 2026 07:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens ToGo’s Cython bindings against C-level lifetime and allocation hazards that can manifest as segfaults under heavy geometry extraction / GC pressure. It primarily shifts wrapper creation toward cloning TG objects (so Python wrappers own their C pointers), adds defensive NULL checks, and improves exception-safety/cleanup around C allocations and GEOS operations.

Changes:

  • Clone borrowed TG pointers before returning Python wrapper objects; add NULL-pointer and clone-failure checks.
  • Make several C allocation/write paths exception-safe via try/finally, plus checked Python→C count conversion for TG APIs.
  • Add regression tests covering wrapper survival after parent GC and empty-geometry meters-grid round trips.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
togo.pyx Adds pointer-cloning/NULL checks and improves cleanup/exception-safety across multiple TG/GEOS interop paths.
tests/test_poly.py Adds a GC regression test ensuring Poly.exterior remains usable after parent collection.
tests/test_geometry.py Adds GC regression tests for accessors and verifies meters-grid conversions for empty geometries.

Comment thread togo.pyx Outdated
Comment thread togo.pyx
Comment thread togo.pyx
Comment thread togo.pyx
Comment thread togo.pyx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

Comment thread togo.pyx
Comment thread togo.pyx
Comment thread togo.pyx
Comment thread togo.pyx
Comment thread togo.pyx
Comment thread togo.pyx
mindflayer and others added 4 commits June 8, 2026 10:38
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread togo.pyx Outdated
Comment thread togo.pyx
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
@mindflayer mindflayer merged commit c66deae into main Jun 8, 2026
7 checks passed
@mindflayer mindflayer deleted the segfault-tackling branch June 8, 2026 09:12
Copilot stopped work on behalf of mindflayer due to an error June 8, 2026 09:12
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.

3 participants