PyO3 / PyO3/pyo3

"lazy" state of `PyErr` ... has challenges

Open
#4,584 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
16.2k
Forks
1k
Avg merge
2d 6h
Merged PRs (30d)
66

Description

PyErr contains some arcane trickery internally to avoid creating a Python exception object, by making some "lazy" state which boxes a PyErrArguments object.

While the original design predates me, I believe that it was done for performance to avoid creating a Python exception object inside of pure-Rust code.

I think there might be a few reasons to revisit this design.

  1. I think the performance motivation is not clear-cut; it makes PyO3's error handling relatively expensive because every error pathway has to box up the state into a dyn Trait before then creating a Python exception.
  2. I think we've learned that a better design is to allow APIs to be generic on the error type where relevant, i.e. in IntoPyObject (and I guess we could do the same in FromPyObject). The proc macros have for a long time allowed any error type which is convertible to PyErr, too.
  3. I realize as I write this issue that PyErrState::normalize is not sound under the freethreaded build: it uses the GIL for synchronization.
  4. The boxed PyErrArguments state is not traversible by the Python GC. In theory this means that the state can contain arbitrary Python objects, could therefore participate cycles and cause memory leaks. We would need to make breaking changes to the trait to fix this.

I propose that instead we remove the ability for PyErr to be lazy and make it a thin wrapper around Py<PyBaseException>. This is essentially the direction that Python went in 3.12 for the global SetRaisedException ffi calls.

It would have the effect of creating a PyErr to change performance profile; instead of creating a boxed lazy exception state it would immediately create the Python object. This might be faster, it might be slower. But I think it would simplify things which might overall improve things.

Due to point 3, i.e. that it's not threadsafe, I'm tempted to just try to land this ASAP on 0.23. cc @ngoldbaum

I could at least make a branch and see what the benchmarks say.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the PyErr lazy state and PyErrState::normalize implementation, along with the IntoPyObject and FromPyObject error paths and related proc-macro handling. Compare the proposed thin Py design on a branch, then run the available benchmarks and check freethreaded synchronization and Python GC behavior. Done means the lazy state is removed without regressions.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, rust
Domain
api, backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.