HyukjinKwon commented on PR #100:
URL:
https://github.com/apache/spark-connect-rust/pull/100#issuecomment-5630166503
**Evidence / pointers for the "no-op under pyo3 0.28" finding above.**
Version pin (this repo):
- `Cargo.toml:87` — `pyo3 = { version = "0.28", features =
["extension-module", "abi3-py39"] }`
- `Cargo.lock` → `pyo3 0.28.3`. Semver `"0.28"` = `>=0.28.0, <0.29`, and the
opt-out default holds for all of 0.28.x.
The change (this PR): `crates/pyspark-rs/src/lib.rs:91`, `#[pymodule] fn
_pyspark(...)` — the **function-style** `#[pymodule]`, so the codegen path is
`pymodule_function_impl`, not the `mod`-style one.
pyo3 0.28.3 codegen — the attribute defaults to `false` when omitted:
- `pyo3-macros-backend-0.28.3/src/module.rs:452` (inside
`pymodule_function_impl`, defined at `module.rs:437`):
```rust
let gil_used = options.gil_used.is_some_and(|op| op.value.value);
```
`Option::is_some_and` returns `false` when the option is `None` (attribute
absent) → `gil_used = false`. (The `mod`-style path at `module.rs:392` is
identical.)
- `pyo3-0.28.3/src/impl_/pymodule.rs:209` `with_gil_used`:
```rust
pub const fn with_gil_used(self, gil_used: bool) -> Self {
#[cfg(Py_3_13)] // line 210
{ self.push(ffi::Py_mod_gil,
if gil_used { ffi::Py_MOD_GIL_USED } // 215
else { ffi::Py_MOD_GIL_NOT_USED }) } // 217
#[cfg(not(Py_3_13))] // line 222
{ let _ = gil_used; self } // no slot emitted at all
}
```
⇒ absent attribute ≡ `gil_used = false` ⇒ `Py_MOD_GIL_NOT_USED` on 3.13+
builds — identical to what this PR writes explicitly.
Why the default flipped (authoritative):
- CHANGELOG `pyo3-0.28.3/CHANGELOG.md:49`, under `[0.28.0]`: "Support for
free-threaded Python is now opt-out rather than opt-in.
[#5564](https://github.com/PyO3/pyo3/pull/5564)"
- Migration guide `pyo3-0.28.3/guide/src/migration.md:8-16` ("Default to
supporting free-threaded Python") — rendered:
https://pyo3.rs/v0.28.3/migration#default-to-supporting-free-threaded-python :
> "Modules now automatically allow use on free-threaded Python, unless
they directly state they require the GIL with `#[pymodule(gil_used = true)]`."
Why it's fully inert on the shipped wheels: the wheels are `cp39-abi3`
(`pyproject.toml:73`, `features = ["pyo3/abi3-py39", ...]`). Under the 3.9
limited API the `Py_3_13` cfg is unset, so `with_gil_used` takes the
`#[cfg(not(Py_3_13))]` branch at `pymodule.rs:222` and emits **no**
`Py_mod_gil` slot regardless of the attribute value.
pyo3 source links (v0.28.3 tag):
- module.rs:
https://github.com/PyO3/pyo3/blob/v0.28.3/pyo3-macros-backend/src/module.rs
- pymodule.rs:
https://github.com/PyO3/pyo3/blob/v0.28.3/src/impl_/pymodule.rs
- migration.md:
https://github.com/PyO3/pyo3/blob/v0.28.3/guide/src/migration.md
Reproduce locally: `cargo check -p pyspark-rs` (passes; the crate builds
`cp39-abi3` per `pyproject.toml`). The codegen facts above are read directly
from the vendored crate under
`~/.cargo/registry/src/*/pyo3-macros-backend-0.28.3/` and `.../pyo3-0.28.3/`.
Thread-safety audit backing the "structurally sound" note: no matches for
`static mut`, `unsendable`, `RefCell`/`Cell<`, `Rc<`, or `thread_local` under
`crates/pyspark-rs/src/`; process-global state is `session.rs:201-204`
(`OnceLock<Mutex<Option<SparkSession>>>`), `session.rs:213`
(`std::sync::Once`), `observation.rs:8` (`AtomicU64`); transport uses
`Arc<Notify>` / `Arc<AtomicBool>` (`transport.rs:70-71,150-151`).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
