Skip to content

Commit d3ce6c7

Browse files
jelle-openaimiss-islington
authored andcommitted
gh-159739: Guard cProfile against reentry while processing an external timer (GH-160405)
An external timer can run Python again when the profiler converts or releases its result or reports an unraisable exception. Calling clear() or disable() at that point could free a profiling context still in use. Calling Profiler.__init__() from the timer could also destroy the active callable. Keep the guard active through result handling and reject reinitialization while an external timer is running. (cherry picked from commit c2b7e0a) Co-authored-by: Jelle Zijlstra <jelle@openai.com>
1 parent e0a3c81 commit d3ce6c7

4 files changed

Lines changed: 58 additions & 15 deletions

File tree

‎Lib/test/test_cprofile.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,39 @@ def __call__(self):
8484
profiler_with_evil_timer.clear()
8585
self.assertEqual(cm.unraisable.exc_type, RuntimeError)
8686

87+
def test_external_timer_result_reentry(self):
88+
import _lsprof
89+
90+
class Ticks:
91+
def __index__(self):
92+
profiler.clear()
93+
return 0
94+
95+
profiler = _lsprof.Profiler(Ticks, timeunit=1)
96+
with support.catch_unraisable_exception() as cm:
97+
profiler.enable()
98+
try:
99+
(lambda: None)()
100+
finally:
101+
profiler.disable()
102+
self.assertIs(cm.unraisable.exc_type, RuntimeError)
103+
104+
def test_external_timer_reinitializes_profiler(self):
105+
import _lsprof
106+
107+
def timer():
108+
profiler.__init__()
109+
return 0
110+
111+
profiler = _lsprof.Profiler(timer)
112+
with support.catch_unraisable_exception() as cm:
113+
profiler.enable()
114+
try:
115+
(lambda: None)()
116+
finally:
117+
profiler.disable()
118+
self.assertIs(cm.unraisable.exc_type, RuntimeError)
119+
87120
def test_enable_in_external_timer(self):
88121
# gh-157639: Enabling the profiler from an external timer should not crash
89122
import _lsprof
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix crashes in :mod:`profiling.tracing` when a custom timer reinitializes the
2+
profiler or its result clears or disables the profiler during conversion or
3+
cleanup.

‎Modules/_lsprof.c‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ typedef struct {
5252
ProfilerContext *currentProfilerContext;
5353
ProfilerContext *freelistProfilerContext;
5454
int flags;
55+
unsigned int activeTimers; /* may overlap if the profiler lock is released */
5556
PyObject *externalTimer;
5657
double externalTimerUnit;
5758
int tool_id;
@@ -63,7 +64,6 @@ typedef struct {
6364
#define POF_ENABLED 0x001
6465
#define POF_SUBCALLS 0x002
6566
#define POF_BUILTINS 0x004
66-
#define POF_EXT_TIMER 0x008
6767
#define POF_NOMEMORY 0x100
6868

6969
/*[clinic input]
@@ -92,13 +92,7 @@ _lsprof_get_state(PyObject *module)
9292

9393
static PyTime_t CallExternalTimer(ProfilerObject *pObj)
9494
{
95-
PyObject *o = NULL;
96-
97-
// External timer can do arbitrary things so we need a flag to prevent
98-
// horrible things to happen
99-
pObj->flags |= POF_EXT_TIMER;
100-
o = _PyObject_CallNoArgs(pObj->externalTimer);
101-
pObj->flags &= ~POF_EXT_TIMER;
95+
PyObject *o = _PyObject_CallNoArgs(pObj->externalTimer);
10296

10397
if (o == NULL) {
10498
PyErr_FormatUnraisable("Exception ignored while calling "
@@ -132,7 +126,12 @@ static inline PyTime_t
132126
call_timer(ProfilerObject *pObj)
133127
{
134128
if (pObj->externalTimer != NULL) {
135-
return CallExternalTimer(pObj);
129+
/* Converting or releasing the result and reporting an unraisable
130+
exception can all run Python code. */
131+
pObj->activeTimers++;
132+
PyTime_t result = CallExternalTimer(pObj);
133+
pObj->activeTimers--;
134+
return result;
136135
}
137136
else {
138137
PyTime_t t;
@@ -364,7 +363,7 @@ ptrace_enter_call(PyObject *self, void *key, PyObject *userObj)
364363

365364
/* Events raised by the external timer must be ignored: it can run
366365
arbitrary code while a context is still being unwound. */
367-
if (pObj->flags & POF_EXT_TIMER) {
366+
if (pObj->activeTimers) {
368367
return;
369368
}
370369

@@ -410,7 +409,7 @@ ptrace_leave_call(PyObject *self, void *key)
410409
ProfilerEntry *profEntry;
411410
ProfilerContext *pContext;
412411

413-
if (pObj->flags & POF_EXT_TIMER) {
412+
if (pObj->activeTimers) {
414413
return;
415414
}
416415

@@ -895,7 +894,7 @@ static PyObject *
895894
_lsprof_Profiler_disable_impl(ProfilerObject *self)
896895
/*[clinic end generated code: output=838cffef7f651870 input=f7e4787cae20f7f6]*/
897896
{
898-
if (self->flags & POF_EXT_TIMER) {
897+
if (self->activeTimers) {
899898
PyErr_SetString(PyExc_RuntimeError,
900899
"cannot disable profiler in external timer");
901900
return NULL;
@@ -955,7 +954,7 @@ static PyObject *
955954
_lsprof_Profiler_clear_impl(ProfilerObject *self)
956955
/*[clinic end generated code: output=dd1c668fb84b1335 input=4aab219d5d7a9bec]*/
957956
{
958-
if (self->flags & POF_EXT_TIMER) {
957+
if (self->activeTimers) {
959958
PyErr_SetString(PyExc_RuntimeError,
960959
"cannot clear profiler in external timer");
961960
return NULL;
@@ -998,6 +997,7 @@ profiler_dealloc(PyObject *op)
998997
}
999998

1000999
/*[clinic input]
1000+
@critical_section
10011001
_lsprof.Profiler.__init__ as profiler_init
10021002
10031003
timer: object(c_default='NULL') = None
@@ -1016,8 +1016,13 @@ is, in seconds).
10161016
static int
10171017
profiler_init_impl(ProfilerObject *self, PyObject *timer, double timeunit,
10181018
int subcalls, int builtins)
1019-
/*[clinic end generated code: output=ac523803ec9f9df2 input=8285ca746f96a414]*/
1019+
/*[clinic end generated code: output=ac523803ec9f9df2 input=5add5fc72beb63ba]*/
10201020
{
1021+
if (self->activeTimers) {
1022+
PyErr_SetString(PyExc_RuntimeError,
1023+
"cannot initialize profiler in external timer");
1024+
return -1;
1025+
}
10211026
if (setSubcalls(self, subcalls) < 0 || setBuiltins(self, builtins) < 0) {
10221027
return -1;
10231028
}

‎Modules/clinic/_lsprof.c.h‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)