Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ cocoa=ON
davix=OFF
minuit2_omp=OFF
x11=OFF
ROOT_CTEST_CUSTOM_FLAGS=-R ^pyunittests-bindings-cppjit-
21 changes: 4 additions & 17 deletions .github/workflows/root-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,10 @@
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

Expand All @@ -132,7 +131,7 @@
- self-hosted
- ${{ matrix.platform }}

name: |

Check failure on line 134 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files

property "overrides" is not defined in object type {arch: string; is_special: bool; platform: string}
${{ matrix.platform }} ${{ matrix.arch }}
${{ (github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' && join( matrix.overrides, ', ' )) || '' }}

Expand All @@ -151,8 +150,8 @@
if: github.event_name == 'pull_request'
env:
GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }}
OVERRIDES: ${{ join( matrix.overrides, ' ') }}

Check failure on line 153 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files

property "overrides" is not defined in object type {arch: string; is_special: bool; platform: string}
run: |

Check failure on line 154 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files

"github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
[ -d "${VIRTUAL_ENV_DIR}" ] && source ${VIRTUAL_ENV_DIR}/bin/activate
echo "Python is now $(which python3) $(python3 --version)"
src/.github/workflows/root-ci-config/build_root.py \
Expand Down Expand Up @@ -184,7 +183,7 @@
- name: Nightlies and push to branch
if: github.event_name == 'schedule' || github.event_name == 'push'
env:
OVERRIDES: ${{ github.event_name == 'push' && format('{0} {1} {2}', '--overrides', env.GLOBAL_OVERRIDES, join( matrix.overrides, ' ')) || '' }}

Check failure on line 186 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files

property "overrides" is not defined in object type {arch: string; is_special: bool; platform: string}
BASE_REF: ${{ github.event_name == 'push' && github.ref_name || inputs.ref_name }}
# For pushes, build exactly the pushed commit, not wherever the
# branch happens to be when the job starts (issue #14061).
Expand Down Expand Up @@ -246,13 +245,7 @@
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
Expand Down Expand Up @@ -372,13 +365,7 @@
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
Expand Down
28 changes: 22 additions & 6 deletions bindings/pyroot/cppjit/src/cpyrt/MemoryRegulator.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ using namespace cppjit;
#include "ProxyWrappers.h"

// Standard
#include <algorithm>
#include <assert.h>
#include <iostream>
#include <string.h>
Expand Down Expand Up @@ -199,10 +200,8 @@ bool cpyrt::MemoryRegulator::UnregisterPyObject(CPPInstance* pyobj,

interop::TCppObject_t cppobj =
pyobj->IsSmart() ? pyobj->GetObjectRaw() : pyobj->GetObject();
if (!cppobj)
return false;

if (unregisterHook) {
if (cppobj && unregisterHook) {
auto res = unregisterHook(cppobj, ((CPPClass*)pyclass)->fCppType);
if (!res.second)
return res.first;
Expand All @@ -212,9 +211,26 @@ bool cpyrt::MemoryRegulator::UnregisterPyObject(CPPInstance* pyobj,
if (!cppobjs)
return false;

// erase if tracked
if (cppobjs->erase(cppobj)) {
pyobj->fFlags &= ~CPPInstance::kIsRegulated;
// The proxy is normally tracked under the address it holds now. That
// address can have changed since registration, though: a function taking
// the object as T** (or void**) may delete it and write a new object into
// the proxy, and cross-inheritance can null the held pointer. Only erase an
// entry that belongs to this proxy, because another proxy may be tracked
// under the new address by now, and otherwise search the map, because an
// entry left behind under the old address would dangle once this proxy is
// gone: a later C++ object at that address would be matched with the dead
// proxy, and re-registering the address writes into its freed memory.
auto ppo = cppobj ? cppobjs->find(cppobj) : cppobjs->end();
if (ppo == cppobjs->end() || ppo->second != (PyObject*)pyobj) {
ppo = std::find_if(cppobjs->begin(), cppobjs->end(),
[pyobj](const CppToPyMap_t::value_type& entry) {
return entry.second == (PyObject*)pyobj;
});
}

pyobj->fFlags &= ~CPPInstance::kIsRegulated;
if (ppo != cppobjs->end()) {
cppobjs->erase(ppo);
return true;
}

Expand Down
41 changes: 35 additions & 6 deletions bindings/pyroot/cppjit/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
# ACLiC into the test working directory (the suite's own Makefile is disabled
# via CPPJIT_TEST_SKIP_MAKE).

set(pythonpaths ${localruntimedir} $ENV{PYTHONPATH})
# The test directory is on the path for the root_ci_diagnostics pytest plugin.
set(pythonpaths ${localruntimedir} ${CMAKE_CURRENT_SOURCE_DIR} $ENV{PYTHONPATH})
cmake_path(CONVERT "${pythonpaths}" TO_NATIVE_PATH_LIST pythonpaths_native)

set(common_env PYTHONPATH=${pythonpaths_native} CPPJIT_TEST_SKIP_MAKE=1)
Expand All @@ -35,11 +36,25 @@ elseif(MSVC)
-e "gSystem->AddLinkedLibs(\"${Python3_LIBRARIES}\")")
endif()

# CRASH_DIAGNOSTICS loads root_ci_diagnostics.py (next to this file) as a
# pytest plugin and runs the test under Python's debug allocator hooks, to get
# an actionable report out of a crash that only happens in CI. See the plugin's
# docstring for what it does and why.
function(CPPJIT_ADD_PYTEST name)
cmake_parse_arguments(ARG
"DROP_ROOTMAP" "" "DICTS;FIXTURES_SETUP;FIXTURES_REQUIRED" ${ARGN})
"DROP_ROOTMAP;CRASH_DIAGNOSTICS" "SUFFIX"
"DICTS;FIXTURES_SETUP;FIXTURES_REQUIRED;ENV;PYTEST_ARGS" ${ARGN})

set(extra)
set(env ${common_env})
set(pytest_args)
if(ARG_CRASH_DIAGNOSTICS)
list(APPEND pytest_args -p root_ci_diagnostics)
list(APPEND env PYTHONMALLOC=debug)
endif()
# TEMP: per-test overrides; later entries win in the test driver
list(APPEND env ${ARG_ENV})
list(APPEND pytest_args ${ARG_PYTEST_ARGS})
if(ARG_DICTS)
set(copy_files)
set(precmd_args)
Expand All @@ -60,17 +75,17 @@ 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
# patches fix many of them on the ROOT stack, so an xpass must not fail
# the suite (pyproject.toml sets xfail_strict=true for upstream CI).
COMMAND ${Python3_EXECUTABLE} -B -m pytest -v -p no:cacheprovider
-o xfail_strict=false
-o xfail_strict=false ${pytest_args}
${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
WORKING_DIR ${CMAKE_CURRENT_BINARY_DIR}
ENVIRONMENT ${common_env}
ENVIRONMENT ${env}
LABELS python
PYTHON_DEPS pytest
${extra})
Expand Down Expand Up @@ -113,8 +128,22 @@ CPPJIT_ADD_PYTEST(test_aclassloader
FIXTURES_REQUIRED cppjit_example01_dict_fixture)
CPPJIT_ADD_PYTEST(test_basic_api
FIXTURES_REQUIRED cppjit_example01_dict_fixture)
CPPJIT_ADD_PYTEST(test_datatypes
# Sporadic crash in the garbage collector during test26_function_pointers on
# the mac-beta ARM64 CI runner; collect diagnostics until it is understood.
CPPJIT_ADD_PYTEST(test_datatypes CRASH_DIAGNOSTICS
FIXTURES_REQUIRED cppjit_datatypes_dict_fixture)
# TEMP: 700 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. Production allocator (the debug allocator hides the bug),
# a full gc.collect() after every test, and the plugin's watch on pytest's
# TestReport objects, which the crash was seen to hit: their reference counts
# are checked around every test, and a finalizer reports the stack of the
# decrement that deallocates one while the stats lists still hold it.
foreach(i RANGE 1 700)
CPPJIT_ADD_PYTEST(test_datatypes CRASH_DIAGNOSTICS SUFFIX -rc${i}
ENV PYTHONMALLOC=default
FIXTURES_REQUIRED cppjit_datatypes_dict_fixture)
endforeach()

CPPJIT_ADD_PYTEST(test_api)
CPPJIT_ADD_PYTEST(test_boost)
Expand Down
Loading
Loading