Repository navigation
[cppjit] Do not throw out of callback wrappers on Apple ARM64 - #23741
Draft
guitargeek wants to merge 2 commits into
Draft
guitargeek wants to merge 2 commits into
guitargeek wants to merge 2 commits into
Conversation
When a Python callable that was passed to C++ as a function pointer, a
std::function or a virtual override fails, the JIT-compiled wrapper around
it throws a cpyrt::PyException. That exception unwinds through the C++
frames in between, which are JIT-compiled or come from a library built by a
different compiler, up to the try block in CPPMethod::ExecuteFast. On
Windows this unwind hangs and the wrapper instead leaves the Python error
set, to be picked up after the C++ call returns.
Exceptions through JIT-compiled frames do not work on Apple ARM64 either:
the test suite skips the tests that catch a PyException in C++ there
("JIT exceptions can not be caught in JITed code on Mac ARM"), and
test27/test28 of test_datatypes, which drive this path many times, are
marked as crashing on macOS. test26_function_pointers drives it exactly
once, and it is the test during which test_datatypes crashes sporadically
in the garbage collector on the mac-beta ARM64 CI runner (and nowhere
else). This is a guess at that crash: use the Windows mechanism on Apple
ARM64 as well.
The platform choice moves into one macro, CPYRT_CALLBACK_NOTHROW, used by
both throw sites in the generated wrapper code (the argument conversion
failure path used to throw on Windows too) and by the post-call check in
ExecuteFast. That check now also sets the kPyException flag, so the error
counts as the callee's for overload resolution and keeps its type instead
of being wrapped in a TypeError about failed overloads.
Verified on Linux with the no-throw path forced on: 24 of the 26 cppjit
test files pass, including test_datatypes with its callable tests; the two
that fail, test_crossinheritance test08 and test_concurrent test04, are the
ones where C++ catches the PyException itself, which is exactly what this
mode gives up and what the suite already skips on Mac ARM. With the
intended platform condition the full cppjit suite passes on Linux.
🤖 Done with the help of AI
Measurement commit for the sporadic crash of test_datatypes on the mac-beta ARM64 runner: disable the Windows and Linux jobs, drop the other macOS platforms from the matrix, and restrict ctest on mac-beta to the cppjit tests with --repeat until-fail:20. At the observed rate of roughly one crash in six runs, 20 repetitions of test_datatypes fail with near certainty without the fix and pass with it. Not to be merged. 🤖 Done with the help of AI
Test Results26 tests 26 ✅ 2m 42s ⏱️ Results for commit 1dc38fb. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a Python callable that was passed to C++ as a function pointer, a std::function or a virtual override fails, the JIT-compiled wrapper around it throws a cpyrt::PyException. That exception unwinds through the C++ frames in between, which are JIT-compiled or come from a library built by a different compiler, up to the try block in CPPMethod::ExecuteFast. On Windows this unwind hangs and the wrapper instead leaves the Python error set, to be picked up after the C++ call returns.
Exceptions through JIT-compiled frames do not work on Apple ARM64 either: the test suite skips the tests that catch a PyException in C++ there ("JIT exceptions can not be caught in JITed code on Mac ARM"), and test27/test28 of test_datatypes, which drive this path many times, are marked as crashing on macOS. test26_function_pointers drives it exactly once, and it is the test during which test_datatypes crashes sporadically in the garbage collector on the mac-beta ARM64 CI runner (and nowhere else). This is a guess at that crash: use the Windows mechanism on Apple ARM64 as well.
The platform choice moves into one macro, CPYRT_CALLBACK_NOTHROW, used by both throw sites in the generated wrapper code (the argument conversion failure path used to throw on Windows too) and by the post-call check in ExecuteFast. That check now also sets the kPyException flag, so the error counts as the callee's for overload resolution and keeps its type instead of being wrapped in a TypeError about failed overloads.
Verified on Linux with the no-throw path forced on: 24 of the 26 cppjit test files pass, including test_datatypes with its callable tests; the two that fail, test_crossinheritance test08 and test_concurrent test04, are the ones where C++ catches the PyException itself, which is exactly what this mode gives up and what the suite already skips on Mac ARM. With the intended platform condition the full cppjit suite passes on Linux.
🤖 Done with the help of AI