Skip to content

Commit 1a38fba

Browse files
committed
Protect against deregistered profile functions in greenlet switches
When greenlet tracking is enabled it's possible that we run into a situation where the function that recreates the Python stack in our TLS variable after a greenlet switch is called **after** the profile function has been deactivated. In this case, recreating the Python stack is wrong as we are no longer tracking POP/PUSH events so when the stack is inspected later nothing guarantees that the frames are still valid. Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
1 parent aa1b452 commit 1a38fba

File tree

5 files changed

+82
-3
lines changed

5 files changed

+82
-3
lines changed

news/700.bugfix.rst

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a crash when a greenlet switch happens after Memray's profile function has been deactivated or replaced.

requirements-test.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Cython
22
coverage[toml]
3-
greenlet; python_version < '3.13'
3+
greenlet; python_version < '3.14'
44
pytest
55
pytest-cov
66
ipython

setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ def build_js_files(self):
113113

114114
test_requires = [
115115
"Cython",
116-
"greenlet; python_version < '3.13'",
116+
"greenlet; python_version < '3.14'",
117117
"pytest",
118118
"pytest-cov",
119119
"ipython",

src/memray/_memray/tracking_api.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -1216,9 +1216,17 @@ Tracker::beginTrackingGreenlets()
12161216
void
12171217
Tracker::handleGreenletSwitch(PyObject* from, PyObject* to)
12181218
{
1219+
// We must stop tracking the stack once our trace function is uninstalled.
1220+
// Otherwise, we'd keep referencing frames after they're destroyed.
1221+
PyThreadState* ts = PyThreadState_Get();
1222+
if (ts->c_profilefunc != PyTraceFunction) {
1223+
return;
1224+
}
1225+
12191226
// Grab the Tracker lock, as this may need to write pushes/pops.
12201227
std::unique_lock<std::mutex> lock(*s_mutex);
12211228
RecursionGuard guard;
1229+
12221230
PythonStackTracker::get().handleGreenletSwitch(from, to);
12231231
}
12241232

tests/integration/test_greenlet.py

+71-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from tests.utils import filter_relevant_allocations
1111

1212
pytestmark = pytest.mark.skipif(
13-
sys.version_info >= (3, 12), reason="Greenlet does not yet support Python 3.12"
13+
sys.version_info >= (3, 14), reason="Greenlet does not yet support Python 3.14"
1414
)
1515

1616

@@ -194,3 +194,73 @@ def stack(alloc):
194194
assert vallocs[0].tid != vallocs[1].tid != vallocs[6].tid
195195
assert vallocs[0].tid == vallocs[2].tid
196196
assert vallocs[1].tid == vallocs[3].tid == vallocs[4].tid == vallocs[5].tid
197+
198+
199+
def test_uninstall_profile_in_greenlet(tmpdir):
200+
"""Verify that memray handles profile function changes in greenlets correctly."""
201+
# GIVEN
202+
output = Path(tmpdir) / "test.bin"
203+
subprocess_code = textwrap.dedent(
204+
f"""
205+
import greenlet
206+
import sys
207+
208+
from memray import Tracker
209+
from memray._test import MemoryAllocator
210+
211+
def foo():
212+
bar()
213+
allocator.valloc(1024 * 10)
214+
215+
def bar():
216+
baz()
217+
218+
def baz():
219+
sys.setprofile(None)
220+
other.switch()
221+
222+
def test():
223+
allocator.valloc(1024 * 70)
224+
main_greenlet.switch()
225+
226+
227+
allocator = MemoryAllocator()
228+
output = "{output}"
229+
230+
with Tracker(output):
231+
main_greenlet = greenlet.getcurrent()
232+
other = greenlet.greenlet(test)
233+
foo()
234+
235+
"""
236+
)
237+
238+
# WHEN
239+
subprocess.run([sys.executable, "-Xdev", "-c", subprocess_code], timeout=5)
240+
241+
# THEN
242+
reader = FileReader(output)
243+
records = list(reader.get_allocation_records())
244+
vallocs = [
245+
record
246+
for record in filter_relevant_allocations(records)
247+
if record.allocator == AllocatorType.VALLOC
248+
]
249+
250+
def stack(alloc):
251+
return [frame[0] for frame in alloc.stack_trace()]
252+
253+
# Verify allocations and their stack traces (which should be empty
254+
# because we remove the tracking function)
255+
assert len(vallocs) == 2
256+
257+
assert stack(vallocs[0]) == []
258+
assert vallocs[0].size == 70 * 1024
259+
260+
assert stack(vallocs[1]) == []
261+
assert vallocs[1].size == 10 * 1024
262+
263+
# Verify thread IDs
264+
main_tid = vallocs[0].tid # inner greenlet
265+
outer_tid = vallocs[1].tid # outer greenlet
266+
assert main_tid == outer_tid

0 commit comments

Comments
 (0)