Skip to content

fix: raise instead of segfaulting when OcTree/ColorOcTree.read() gets a bad path - #70

Merged
wkentaro merged 1 commit into
mainfrom
fix/octree-read-null-segfault
Jun 20, 2026
Merged

fix: raise instead of segfaulting when OcTree/ColorOcTree.read() gets a bad path#70
wkentaro merged 1 commit into
mainfrom
fix/octree-read-null-segfault

Conversation

@wkentaro

Copy link
Copy Markdown
Owner

OcTree.read() / ColorOcTree.read() segfaulted instead of raising when handed a missing or unreadable path (#68). octomap's C++ AbstractOcTree::read() returns NULL in that case; the wrappers assigned it straight to thisptr, so the next method call dereferenced NULL and took down the interpreter with SIGSEGV.

Now the wrappers NULL-check after read() and raise a Python exception: FileNotFoundError for a missing path, OSError for an unreadable/corrupt file, and ValueError for bad serialized bytes. Both OcTree and ColorOcTree go through one shared helper.

Test plan

  • octomap.OcTree.read("/nonexistent.ot").getTreeType() now raises FileNotFoundError (exit 0) instead of exit 139
  • regression tests added: test_read_missing_path_raises and test_read_unreadable_file_raises in tests/octomap_test.py and tests/color_octree_test.py
  • uv run pytest tests/ — 40 passed
  • uv run ruff check on the touched tests — clean

Closes #68

@wkentaro wkentaro self-assigned this Jun 20, 2026
@wkentaro wkentaro added this to the v1.10.0.0 milestone Jun 20, 2026
… a bad path

octomap's C++ AbstractOcTree::read() returns NULL when the path is missing
or unreadable. The wrappers assigned that straight to thisptr, so the next
method call dereferenced NULL and crashed the interpreter with SIGSEGV.

NULL-check after read() and raise a Python exception (FileNotFoundError for
a missing path, OSError for an unreadable/corrupt one, ValueError for bad
serialized bytes). Covers both OcTree and ColorOcTree.

Closes #68
@wkentaro
wkentaro force-pushed the fix/octree-read-null-segfault branch from 7bd6636 to 46b6e0c Compare June 20, 2026 13:12
@wkentaro

Copy link
Copy Markdown
Owner Author

This was generated by AI during PR processing.

Verdict: recommend-merge

Rebased onto current main (post-#67) — main's changes overlapped tests/octomap_test.py, so I rebased to get CI against the real merged state; the rebase was conflict-free. CI is now fully green on the rebased commit and merge state is CLEAN.

Verified the fix end-to-end: rebuilt the extension and ran the new read-error tests (OcTree + ColorOcTree, missing path and corrupt file) — all pass. The NULL-pointer segfault is now turned into FileNotFoundError (missing path, via os.stat), OSError "failed to read" (corrupt-but-existing file), and ValueError (bad serialized bytes).

Code review found no correctness issues: _raise_read_error is correctly module-level, fsencode/fsdecode round-trips, the error branching matches the tests, and there's no double-free (__dealloc__ is guarded by if self.thisptr:).

Ready for your review and merge.

@wkentaro wkentaro added the recommend-merge pr: Agent finalized and endorses it: review and merge label Jun 20, 2026
@wkentaro
wkentaro merged commit 1e24464 into main Jun 20, 2026
@wkentaro
wkentaro deleted the fix/octree-read-null-segfault branch June 20, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

recommend-merge pr: Agent finalized and endorses it: review and merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OcTree/ColorOcTree.read() segfaults on a nonexistent or unreadable path

1 participant