Skip to content

Commit 39eeb7d

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 9a7f3e7 commit 39eeb7d

2 files changed

Lines changed: 14 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: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,18 @@ def func(x, pars):
6565
self.assertTrue(r.IsValid())
6666
self.assertGreater(r.Parameter(0), 0)
6767

68+
def test_fit_with_warning(self):
69+
"""
70+
Regression test for https://github.com/root-project/root/issues/22396
71+
"""
72+
import ROOT
73+
74+
h = ROOT.TH1D("test", "test", 100, 0, 1)
75+
# Trying to fit a empty histogram will result in a warning on the C++ side which is re-raised as a Python
76+
# warning by the ROOT CPython extension
77+
with self.assertWarnsRegex(RuntimeWarning, "Fit data is empty"):
78+
h.Fit("gaus", "S")
79+
6880

6981
if __name__ == "__main__":
7082
unittest.main()

0 commit comments

Comments
 (0)