eagerly normalize fetched exception on Python 3.11 and older

[davidhewitt]

Related to #4584

The idea here is to reduce the amount of lazy normalization that might be going on while also simplifying our code to be able to always treat fetched exceptions as normalized.

This is already the case for Python 3.12 (exceptions are stored as a single exception value in normalized form), but in 3.11 and older it is not guaranteed. This PR introduces a normalization step to those older Pythons as part of PyErr::take().

It is admittedly a slight performance regression on those older Pythons (I measure maybe ~5ns cost to the err_new_restore_and_fetch benchmark on my machine), however:

• it's only a small performance loss in an error-handling pathway
• I think the simplification is worth it

[davidhewitt]

@alex, as this would affect all Python versions of cryptography, I wonder what you think of this change? I assume that 0.23 is sufficiently breaking that it'll be very hard to test this change against cryptography CI 😬

[alex]

Heh, not easily. 46 previous errors; 317 warnings emitted :D

The performance impact here would be for cases where a PyErr is created, but doesn't actually get "raised" into Python, right? I'm not sure we really do that ever, so I'm not especially concerned.

[davidhewitt]

Not exactly. More specifically, this inserts an extra FFI call whenever we fetch a PyErr from the interpreter, to ensure that exception is in the normalized form. I think most cases the exception will be normalized eventually anyway (e.g. when it is caught). If the exception is already normalized, this FFI call will be essentially a noop.

The only case where this might lead to some wasted work would be if you have patterns like if let Ok(x) = do_some_fallible_thing_creating_pyresult(), throwing away the possible error where it happens to be that do_some_fallible_thing_creating_pyresult() created un-normalized exceptions; this PR would now wastefully normalize it (basically create the full exception object).

[alex]

Hard to be certain, but I don't think we have that pattern often, if at all

…

On Sun, Oct 27, 2024, 4:03 PM David Hewitt ***@***.***> wrote: Not exactly. More specifically, this inserts an extra FFI call whenever we fetch a PyErr from the interpreter, to ensure that exception is in the normalized form. I think most cases the exception will be normalized eventually anyway (e.g. when it is caught). If the exception is already normalized, this FFI call will be essentially a noop. The only case where this might lead to some wasted work would be if you have patterns like if let Ok(x) = do_some_fallible_thing_creating_pyresult(), throwing away the possible error where it happens to be that do_some_fallible_thing_creating_pyresult() created un-normalized exceptions; this PR would now wastefully normalize it (basically create the full exception object). — Reply to this email directly, view it on GitHub <#4655 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBCOFCYJPHIBK2MTXOLZ5U2HZAVCNFSM6AAAAABQUNWNYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBQGE2TMNZZGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[davidhewitt]

In which case, I'm going to merge this, and if we have reports against this in 0.23 I can worry about what to do later 😂

[ngoldbaum]

Heh, not easily. 46 previous errors; 317 warnings emitted :D

@alex BTW, if you need support in getting a version of cryptography that supports PyO3 0.23 and free-threading, my plan is to work on libraries that depend on PyO3 after 0.23 gets shipped and cryptography is near the top of our list of packages we'd like to see working. Please let me know if you'd like some hands-on help or support in other ways.

[alex]

We're definitely always happy to take contributions! I think a lot of these are probably just regexps to remove _bound, but some need effort:-)

…

On Wed, Oct 30, 2024, 3:21 PM Nathan Goldbaum ***@***.***> wrote: Heh, not easily. 46 previous errors; 317 warnings emitted :D @alex <https://github.com/alex> BTW, if you need support in getting a version of cryptography that supports PyO3 0.23 and free-threading, my plan is to work on libraries that depend on PyO3 after 0.23 gets shipped and cryptography is near the top of our list of packages we'd like to see working. Please let me know if you'd like some hands-on help or support in other ways. — Reply to this email directly, view it on GitHub <#4655 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBFMZ6LMJQTEJU7MZBLZ6EWVDAVCNFSM6AAAAABQUNWNYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBYGE3DCMBWGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>