From 3312a67ed0aef6c7188b70b62868010113ef1cbb Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 16 Oct 2026 13:52:07 +0000 Subject: [PATCH 1/2] [cppjit] Do not throw out of callback wrappers on Apple ARM64 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. --- .../pyroot/cppjit/src/cpyrt/CPPMethod.cxx | 8 +++++--- bindings/pyroot/cppjit/src/cpyrt/CommonDefs.h | 19 +++++++++++++++++++ bindings/pyroot/cppjit/src/cpyrt/Utility.cxx | 11 ++++++++--- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx index 0221f836748ac..ade34612af2c3 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx @@ -181,10 +181,12 @@ inline PyObject* cpyrt::CPPMethod::ExecuteFast(void* self, ptrdiff_t offset, result = nullptr; } -// TODO: covers the PyException throw case, which does not seem to work on -// Windows, so instead leaves the error be -#ifdef _WIN32 +#if CPYRT_CALLBACK_NOTHROW + // the callback wrappers leave the Python error set instead of throwing a + // PyException (see CommonDefs.h), so pick it up here, as the catch above + // would have, and flag it as the callee's error for overload resolution if (PyErr_Occurred()) { + ctxt->fFlags |= CallContext::kPyException; if (ResultIsPyObject()) Py_XDECREF(result); result = nullptr; diff --git a/bindings/pyroot/cppjit/src/cpyrt/CommonDefs.h b/bindings/pyroot/cppjit/src/cpyrt/CommonDefs.h index 0db73202137e5..17e27fe8bddc2 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CommonDefs.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CommonDefs.h @@ -31,4 +31,23 @@ #endif +// How JIT-compiled callback wrappers (function pointers, std::function, and +// virtual overrides implemented in Python) report a failed Python call. The +// default is to throw a cpyrt::PyException from the wrapper, which unwinds +// through the C++ frames in between (JIT-compiled, or built by another +// compiler) up to the try block in CPPMethod::ExecuteFast. That unwind hangs +// on Windows, and on Apple ARM64 exceptions through JIT-compiled frames are +// known not to work (the test suite skips the tests that catch a PyException +// in C++ there) and are the prime suspect for the sporadic crash of +// test_datatypes on the macOS beta CI runner. On those platforms the wrapper +// therefore leaves the Python error set and returns a default value instead; +// the C++ code keeps running and ExecuteFast turns the pending error into a +// failed call once the C++ call returns. The price is that C++ code can not +// catch the Python error as a PyException. +#if defined(_WIN32) || (defined(__APPLE__) && defined(__aarch64__)) +#define CPYRT_CALLBACK_NOTHROW 1 +#else +#define CPYRT_CALLBACK_NOTHROW 0 +#endif + #endif // !CPYRT_COMMONDEFS_H diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx index 9538a57b46260..13ec44b035abd 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx @@ -1065,7 +1065,12 @@ void cpyrt::Utility::ConstructCallbackPreamble( } code << " } catch(int) {\n" << " for (auto pyarg : pyargs) Py_XDECREF(pyarg);\n" +#if CPYRT_CALLBACK_NOTHROW + // error is set; see CPYRT_CALLBACK_NOTHROW in CommonDefs.h + << (isVoid ? " return;\n" : " return ret;\n") +#else << " cppjit::cpyrt::PyException pyexc; throw pyexc;\n" +#endif << " }\n"; } } @@ -1095,9 +1100,9 @@ void cpyrt::Utility::ConstructCallbackReturn(const std::string& retType, if (isPtr) code << " }\n"; code << " if (!cOk) {" // assume error set when converter failed -// TODO: On Windows, throwing a C++ exception here makes the code hang; leave -// the error be which allows at least one layer of propagation -#ifdef _WIN32 +// see CPYRT_CALLBACK_NOTHROW in CommonDefs.h: leave the error be, which allows +// at least one layer of propagation +#if CPYRT_CALLBACK_NOTHROW " /* do nothing */ }\n" #else " cppjit::cpyrt::PyException pyexc; throw pyexc; }\n" From b4168d4c342271efdfe40677b1bd8a69da6526da Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Sat, 17 Oct 2026 20:52:55 +0000 Subject: [PATCH 2/2] TEMP: [CI] Run only mac-beta, only cppjit tests, 300 copies of test_datatypes 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, restrict ctest on mac-beta to the cppjit tests, and register 300 extra copies of test_datatypes (-rep1 to -rep300) that share the dictionary fixture. ctest runs them 8 at a time in the common test directory, like the real suite runs the cppjit tests, instead of the sequential and otherwise idle repetitions --repeat gives. The crash rate measured over 201 PR jobs and 13 nightlies is about 3%, so 300 clean runs without the fix have a probability below one in a thousand, while 300 runs with it should show no failure. About 6 seconds per copy, so a few minutes of test time. Not to be merged. --- .../root-ci-config/buildconfig/mac-beta.txt | 1 + .github/workflows/root-ci.yml | 21 ++++--------------- bindings/pyroot/cppjit/test/CMakeLists.txt | 11 ++++++++-- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/.github/workflows/root-ci-config/buildconfig/mac-beta.txt b/.github/workflows/root-ci-config/buildconfig/mac-beta.txt index f7de3ebcdb542..c4e6c9003106e 100644 --- a/.github/workflows/root-ci-config/buildconfig/mac-beta.txt +++ b/.github/workflows/root-ci-config/buildconfig/mac-beta.txt @@ -22,3 +22,4 @@ cocoa=ON davix=OFF minuit2_omp=OFF x11=OFF +ROOT_CTEST_CUSTOM_FLAGS=-R ^pyunittests-bindings-cppjit- diff --git a/.github/workflows/root-ci.yml b/.github/workflows/root-ci.yml index 0af636192a117..9477dde216f5a 100644 --- a/.github/workflows/root-ci.yml +++ b/.github/workflows/root-ci.yml @@ -120,11 +120,10 @@ jobs: matrix: # Specify platform + optional build option overrides # Build options: https://root.cern/install/build_from_source/#all-build-options - platform: [mac26, mac27, mac-beta] + # TEMP: only mac-beta, to measure the sporadic cppjit test_datatypes crash + platform: [mac-beta] arch: [ARM64] include: - - platform: mac15 - arch: X86 - platform: mac-beta is_special: true @@ -246,13 +245,7 @@ jobs: build-windows: # For any event that is not a PR, the CI will always run. In PRs, the CI # can be skipped if the tag [skip-ci] or [skip ci] is written in the title. - if: | - (github.repository_owner == 'root-project' && github.event_name != 'pull_request') || - (github.event_name == 'pull_request' && !( - contains(github.event.pull_request.title, '[skip-ci]') || - contains(github.event.pull_request.title, '[skip ci]') || - contains(github.event.pull_request.labels.*.name, 'skip ci') - )) + if: false # TEMP: only mac-beta runs, see build-macos permissions: contents: read @@ -372,13 +365,7 @@ jobs: build-linux: # For any event that is not a PR, the CI will always run. In PRs, the CI # can be skipped if the tag [skip-ci] or [skip ci] is written in the title. - if: | - (github.repository_owner == 'root-project' && github.event_name != 'pull_request') || - (github.event_name == 'pull_request' && !( - contains(github.event.pull_request.title, '[skip-ci]') || - contains(github.event.pull_request.title, '[skip ci]') || - contains(github.event.pull_request.labels.*.name, 'skip ci') - )) + if: false # TEMP: only mac-beta runs, see build-macos permissions: contents: read diff --git a/bindings/pyroot/cppjit/test/CMakeLists.txt b/bindings/pyroot/cppjit/test/CMakeLists.txt index 482c2abb89102..84bf4d10be550 100644 --- a/bindings/pyroot/cppjit/test/CMakeLists.txt +++ b/bindings/pyroot/cppjit/test/CMakeLists.txt @@ -37,7 +37,7 @@ endif() function(CPPJIT_ADD_PYTEST name) cmake_parse_arguments(ARG - "DROP_ROOTMAP" "" "DICTS;FIXTURES_SETUP;FIXTURES_REQUIRED" ${ARGN}) + "DROP_ROOTMAP" "SUFFIX" "DICTS;FIXTURES_SETUP;FIXTURES_REQUIRED" ${ARGN}) set(extra) if(ARG_DICTS) @@ -60,7 +60,7 @@ function(CPPJIT_ADD_PYTEST name) endif() string(REGEX REPLACE "[_]" "-" good_name "${name}") - set(test_name pyunittests-bindings-cppjit-${good_name}) + set(test_name pyunittests-bindings-cppjit-${good_name}${ARG_SUFFIX}) ROOT_ADD_TEST(${test_name} # Upstream marks platform xfails without explicit strict=; the in-tree @@ -115,6 +115,13 @@ CPPJIT_ADD_PYTEST(test_basic_api FIXTURES_REQUIRED cppjit_example01_dict_fixture) CPPJIT_ADD_PYTEST(test_datatypes FIXTURES_REQUIRED cppjit_datatypes_dict_fixture) +# TEMP: 300 more copies of test_datatypes, run by ctest in parallel in this +# directory like the real suite does, to measure its sporadic crash on the +# mac-beta CI runner (about 3% of runs). +foreach(i RANGE 1 300) + CPPJIT_ADD_PYTEST(test_datatypes SUFFIX -rep${i} + FIXTURES_REQUIRED cppjit_datatypes_dict_fixture) +endforeach() CPPJIT_ADD_PYTEST(test_api) CPPJIT_ADD_PYTEST(test_boost)