GH-51229: [Python] Raise instead of crashing on unopened resize - #51246
GH-51229: [Python] Raise instead of crashing on unopened resize#512461fanwang wants to merge 1 commit into
Conversation
Reject resize calls on directly constructed MemoryMappedFile objects before dereferencing the native handle. Generated-by: GitHub Copilot CLI (Claude Opus 5) Signed-off-by: Stefan Wang <1fannnw@gmail.com>
|
|
There was a problem hiding this comment.
🟢 Approval recommended
The change correctly prevents a reproducible segfault by aligning resize() with existing open-state checks, and the regression is covered by a subprocess-based test.
Pull request overview
This PR addresses a Python-level crash in pyarrow.MemoryMappedFile.resize() when MemoryMappedFile() is constructed without opening a file, by adding an open-state guard so the misuse raises a catchable exception instead of segfaulting.
Changes:
- Add
_assert_open()to theMemoryMappedFile.resize()wrapper before calling the native resize implementation. - Add a regression test that exercises the behavior in a subprocess so a segfault would be observable without killing the pytest runner.
File summaries
| File | Description |
|---|---|
| python/pyarrow/io.pxi | Adds an open-state assertion to MemoryMappedFile.resize() to prevent dereferencing a null native handle. |
| python/pyarrow/tests/test_io.py | Adds a subprocess-based regression test ensuring uninitialized MemoryMappedFile().resize() raises ValueError instead of crashing. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| assert f.read() == bytes(arr[:SIZE]) | ||
|
|
||
|
|
||
| def test_memory_map_resize_uninitialized(): |
There was a problem hiding this comment.
Would it make sense to use simple with pytest.raises(ValueError, match="***"): syntax?
Rationale for this change
pa.MemoryMappedFile()can be constructed without opening a file. Calling resize on that object dereferences an empty native handle and takes down the Python process with a SIGSEGV, so an application cannot catch or recover from it.What changes are included in this PR?
The resize path now applies the same open-state check the other file operations already use, before it reaches the native resize call. A directly constructed object raises
ValueError("I/O operation on closed file").Are these changes tested?
Yes. The regression drives the call in a subprocess so a crash is observable rather than fatal to the test run.
Are there any user-facing changes?
Yes. Misuse of a directly constructed memory-mapped file now raises a Python exception instead of terminating the process.
This PR contains a "Critical Fix". It fixes a process crash reachable from ordinary Python-level object state.
MemoryMappedFile().resize()segfaults when no file has been opened #51229