kylebarron commented on code in PR #5070:
URL: https://github.com/apache/arrow-rs/pull/5070#discussion_r1391280121
##########
arrow-schema/src/ffi.rs:
##########
@@ -351,6 +355,9 @@ impl Drop for FFI_ArrowSchema {
}
}
+unsafe impl Send for FFI_ArrowSchema {}
+unsafe impl Sync for FFI_ArrowSchema {}
Review Comment:
No, I can't say I understand Send/Sync well enough to be 100% confident this
is safe.
The motivation is that right now it's impossible to create a `PyCapsule`
instance from an `FFI_ArrowSchema` because it's not `Send`. (See
[`PyCapsule::new`](https://docs.rs/pyo3/latest/pyo3/types/struct.PyCapsule.html#method.new)).
```rs
let ffi_schema = FFI_ArrowSchema::try_from(&*field).unwrap();
let schema_capsule_name = CString::new("arrow_schema").unwrap();
Python::with_gil(|py| {
let capsule = PyCapsule::new(
py,
ffi_schema,
Some(schema_capsule_name),
)
.unwrap();
});
```
gives
```
error[E0277]: `*const i8` cannot be sent between threads safely
--> src/array/point.rs:35:17
|
33 | let capsule = PyCapsule::new(
| -------------- required by a bound introduced
by this call
34 | py,
35 | ffi_schema,
| ^^^^^^^^^^ `*const i8` cannot be sent between threads
safely
|
= help: within `arrow::ffi::FFI_ArrowSchema`, the trait
`std::marker::Send` is not implemented for `*const i8`
note: required because it appears within the type `FFI_ArrowSchema`
-->
/Users/kyle/.cargo/git/checkouts/arrow-rs-53e12341924f0a1c/b1c43a4/arrow-schema/src/ffi.rs:71:12
|
71 | pub struct FFI_ArrowSchema {
| ^^^^^^^^^^^^^^^
note: required by a bound in `pyo3::types::PyCapsule::new`
-->
/Users/kyle/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.20.0/src/types/capsule.rs:78:29
|
78 | pub fn new<T: 'static + Send + AssertNotZeroSized>(
| ^^^^ required by this bound in
`PyCapsule::new`
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
