Skip to content

Commit 56ce5ff

Browse files
committed
[python] Ensure GIL is held during CPython API call
ROOT's CPython extension defines a custom error handler that tries to use Python standard warnings when possible. In such cases, the CPython API is called and thus the GIL must be held. If the calling Python function had previously released the GIL it will result in trouble. This is the case for the TH1.Fit method which has been changed to always releasing the GIL, under the assumption that everything happening during the call happens in C++, but that's not true. This commit proposes to restore the GIL for the call to the CPython API to raise the warning.
1 parent 4f5b5a6 commit 56ce5ff

2 files changed

Lines changed: 16 additions & 0 deletions

File tree

bindings/pyroot/pythonizations/src/RPyROOTApplication.cxx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ static void ErrMsgHandler(int level, Bool_t abort, const char *location, const c
122122
// the GIL.
123123
if (!gGlobalMutex) {
124124
// Either printout or raise exception, depending on user settings
125+
auto state = PyGILState_Ensure();
125126
PyErr_WarnExplicit(NULL, (char *)msg, (char *)location, 0, (char *)"ROOT", NULL);
127+
PyGILState_Release(state);
126128
} else {
127129
::DefaultErrorHandler(level, abort, location, msg);
128130
}

bindings/pyroot/pythonizations/test/th1.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,19 @@ def func(x, pars):
6666
self.assertGreater(r.Parameter(0), 0)
6767

6868

69+
class TH1FitWarning(unittest.TestCase):
70+
def test_fit_with_warning(self):
71+
"""
72+
Regression test for https://github.com/root-project/root/issues/22396
73+
"""
74+
import ROOT
75+
76+
h = ROOT.TH1D("test", "test", 100, 0, 1)
77+
# Trying to fit a empty histogram will result in a warning on the C++ side which is re-raised as a Python
78+
# warning by the ROOT CPython extension
79+
with self.assertWarns(RuntimeWarning, msg="Fit data is empty"):
80+
h.Fit("gaus", "S")
81+
82+
6983
if __name__ == "__main__":
7084
unittest.main()

0 commit comments

Comments
 (0)