bit2swaz opened a new pull request, #10419:
URL: https://github.com/apache/arrow-rs/pull/10419
# Which issue does this PR close?
Closes #10253
# Rationale for this change
`ArrowArrayStreamReader::try_new` is a safe fn, but it calls
`get_stream_schema`, which derefs and calls the caller-supplied `get_schema` fn
pointer. every field of `FFI_ArrowArrayStream` is public, so safe code can
construct a stream with garbage callbacks and hit UB without writing a single
`unsafe` block. the `release.is_none()` check only catches an already-released
stream, not a garbage-populated one
both `from_raw` constructors in the same file are already `unsafe` for this
exact reason. `try_new` was the one missing it.
# What changes are included in this PR?
- `ArrowArrayStreamReader::try_new` is now `unsafe fn` with a `# Safety`
section documenting that the stream's callbacks and `private_data` must be
valid per the C stream interface
- `from_raw` body and the two in-module tests wrap their `try_new` calls in
`unsafe {}`.
- both `try_new` call sites in `arrow-pyarrow` wrap in `unsafe {}` with a
`// SAFETY:` note
- dropped the stale `#[allow(dead_code)]` on `try_new`
the other option from #10253 (private fields on `FFI_ArrowArrayStream`)
would break the `#[repr(C)]` ABI that external FFI producers fill field by
field, without gaining anything — the callbacks still have to be trusted at
call time regardless of visibility.
# Are these changes tested?
yes. a `compile_fail` doctest on `try_new` verifies that calling it without
an `unsafe` block is a compile error. the existing round-trip and error-import
tests cover the happy path and still pass
# Are there any user-facing changes?
yes, this is a breaking change. `ArrowArrayStreamReader::try_new` is now
`unsafe`. callers that obtain a stream from a legitimate FFI source (e.g.
`from_raw`, `_export_to_c`) just need to wrap the call in `unsafe {}`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
