-
Notifications
You must be signed in to change notification settings - Fork 48
Correct management of GIL in concurrency tests #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The GIL should always be acquired via the thread state API when doing Python calls from multithreaded C++. Fixes the test failure with Python debug builds.
|
It's cppyy's task to acquire the GIL where needed. If this test doesn't run as-is, then that means there's a bug in CPyCppyy, not that the test needs to be "fixed." Does this fail more generally, or only in debug builds, or does it accidentally "work" normally, with a state validation failing in debug build? |
|
The latter. The test fails with clear assertion failures about the thread state management. Ok I didn't know that the backend is expected to handle this, but of course that would be more elegant! We have a run the Python debug in the ROOT CI now. I reverted the "fix" in ROOT in a draft PR, so that you can see yourself what happens (the failure will be with |
|
Ok the error is not that clear after all 😆 2025-07-17T18:03:56.6183392Z 680/3136 Test #33: pyunittests-bindings-pyroot-cppyy-cppyy-test-concurrent ...........................................***Failed 2.75 sec
2025-07-17T18:03:56.6183996Z ============================= test session starts ==============================
2025-07-17T18:03:56.6184423Z platform linux -- Python 3.14.0b4, pytest-8.3.5, pluggy-1.6.0 -- /py-venv/ROOT-CI/bin/python3.14
2025-07-17T18:03:56.6184808Z cachedir: .pytest_cache
2025-07-17T18:03:56.6185045Z rootdir: /github/home/ROOT-CI/src/bindings/pyroot/cppyy/cppyy
2025-07-17T18:03:56.6185381Z configfile: pyproject.toml
2025-07-17T18:03:56.6185577Z collecting ... collected 7 items
2025-07-17T18:03:56.6185694Z
2025-07-17T18:03:56.6282793Z ../../../../../../src/bindings/pyroot/cppyy/cppyy/test/test_concurrent.py::TestCONCURRENT::test01_simple_threads SKIPPED
2025-07-17T18:03:56.6283461Z ../../../../../../src/bindings/pyroot/cppyy/cppyy/test/test_concurrent.py::TestCONCURRENT::test02_futures SKIPPED
2025-07-17T18:03:56.6302349Z ../../../../../../src/bindings/pyroot/cppyy/cppyy/test/test_concurrent.py::TestCONCURRENT::test03_timeout PASSED
2025-07-17T18:03:56.6522930Z ../../../../../../src/bindings/pyroot/cppyy/cppyy/test/test_concurrent.py::TestCONCURRENT::test04_cpp_threading_with_exceptions *** Break *** segmentation violation
2025-07-17T18:03:56.6523634Z Generating stack trace...
2025-07-17T18:03:56.6523946Z 0x00007f53391d2bff in _Py_INCREF_IncRefTotal + 0x1a from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6524419Z 0x00007f53392ab68e in <unknown> from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6524800Z 0x00007f53392ab99a in <unknown> from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6525190Z 0x00007f53392abba0 in <unknown> from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6525575Z 0x00007f53392b0559 in PyWeakref_GetRef + 0xd5 from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6526335Z 0x00007f5315ced734 in CPyCppyy::DispatchPtr::Get(bool) const at /github/home/ROOT-CI/src/bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyy.h:366 (discriminator 2) from /github/home/ROOT-CI/build/lib/libcppyy.so
2025-07-17T18:03:56.6527055Z 0x00007f532997c213 in <unknown function>
2025-07-17T18:03:56.6527262Z 0x00007f532600f1a3 in <unknown function>
2025-07-17T18:03:56.6527551Z 0x00007f5336722164 in <unknown> from /lib64/libstdc++.so.6
2025-07-17T18:03:56.6527859Z 0x00007f5338edeca4 in <unknown> from /lib64/libc.so.6
2025-07-17T18:03:56.6528175Z 0x00007f5338f61454 in __clone at :? from /lib64/libc.so.6
2025-07-17T18:03:56.6528815Z *** Break *** segmentation violation
2025-07-17T18:03:56.6529016Z Generating stack trace...
2025-07-17T18:03:56.6529331Z 0x00007f53391d2bff in _Py_INCREF_IncRefTotal + 0x1a from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6529724Z 0x00007f53392ab68e in <unknown> from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6530088Z 0x00007f53392ab99a in <unknown> from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6530445Z 0x00007f53392abba0 in <unknown> from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6530821Z 0x00007f53392b0559 in PyWeakref_GetRef + 0xd5 from /lib64/libpython3.14d.so.1.0
2025-07-17T18:03:56.6531677Z 0x00007f5315ced734 in CPyCppyy::DispatchPtr::Get(bool) const at /github/home/ROOT-CI/src/bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyy.h:366 (discriminator 2) from /github/home/ROOT-CI/build/lib/libcppyy.so
2025-07-17T18:03:56.6532310Z 0x00007f532997c213 in <unknown function>
2025-07-17T18:03:56.6532516Z 0x00007f532600f1a3 in <unknown function>
2025-07-17T18:03:56.6532795Z 0x00007f5336722164 in <unknown> from /lib64/libstdc++.so.6
2025-07-17T18:03:56.6533135Z 0x00007f5338edeca4 in <unknown> from /lib64/libc.so.6
2025-07-17T18:03:56.6533386Z 0x00007f5338f61454 in __clone at :? from /lib64/libc.so.6
2025-07-17T18:03:56.6533771Z CMake Error at /github/home/ROOT-CI/src/cmake/modules/RootTestDriver.cmake:232 (message):
2025-07-17T18:03:56.6534112Z error code: 129
2025-07-17T18:03:56.6534213Z
2025-07-17T18:03:56.6534232Z
2025-07-17T18:03:56.6534236Z Maybe it was better on my local machine, but I remember something clearer. But does that convince you that something needs to be fixed in the backend? |
The GIL should always be acquired via the thread state API when doing Python calls from multithreaded C++.
Fixes the test failure with Python debug builds.
Corresponding to the following ROOT PR: root-project/root#19237