8000 gh-134604: Fix tracemalloc crash with subinterpreters by emmatyping · Pull Request #134667 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-134604: Fix tracemalloc crash with subinterpreters #134667

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

gh-134604: Fix tracemalloc crash with subinterpreters #134667

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
029241e
Delay subinterpreter finalization to after tracemalloc
emmatyping May 25, 2025
f602208
Copy frame filenames in tracemalloc
emmatyping May 25, 2025
daebaff
Add test for subinterpreters + tracemalloc
emmatyping May 25, 2025
d69d5c0
Add NEWS entry
emmatyping May 25, 2025
911318e
Revert "Delay subinterpreter finalization to after tracemalloc"
emmatyping May 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Copy frame filenames in tracemalloc 
With subinterpreters, frames can be destroyed at any point, meaning
tracemalloc needs to copy instead of reference the frame filenames.
  • Loading branch information
@emmatyping
emmatyping committed May 25, 2025
commit f602208cfb48bab78a26d16de105c75e2b2c1a1a
2 changes: 1 addition & 1 deletion Include/internal/pycore_tracemalloc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ __attribute__((packed))
tracemalloc_frame {
/* filename cannot be NULL: "<unknown>" is used if the Python frame
filename is NULL */
PyObject *filename;
char *filename;
unsigned int lineno;
};
#ifdef _MSC_VER
Expand Down
82 changes: 48 additions & 34 deletions Python/tracemalloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,20 @@ set_reentrant(int reentrant)


static Py_uhash_t
hashtable_hash_pyobject(const void *key)
hashtable_hash_filename(const void *key)
{
PyObject *obj = (PyObject *)key;
return PyObject_Hash(obj);
char *obj = (char *)key;
return Py_HashBuffer(obj, strlen(obj));
}


static int
hashtable_compare_unicode(const void *key1, const void *key2)
{
PyObject *obj1 = (PyObject *)key1;
PyObject *obj2 = (PyObject *)key2;
char *obj1 = (char *)key1;
char *obj2 = (char *)key2;
if (obj1 != NULL && obj2 != NULL) {
return (PyUnicode_Compare(obj1, obj2) == 0);
return (strcmp(obj1, obj2) == 0);
}
else {
return obj1 == obj2;
Expand Down Expand Up @@ -209,8 +209,7 @@ hashtable_compare_traceback(const void *key1, const void *key2)
if (frame1->lineno != frame2->lineno) {
return 0;
}
if (frame1->filename != frame2->filename) {
assert(PyUnicode_Compare(frame1->filename, frame2->filename) != 0);
if (strcmp(frame1->filename, frame2->filename) != 0) {
return 0;
}
}
Expand All @@ -222,15 +221,15 @@ static void
tracemalloc_get_frame(_PyInterpreterFrame *pyframe, frame_t *frame)
{
assert(PyStackRef_CodeCheck(pyframe->f_executable));
frame->filename = &_Py_STR(anon_unknown);
frame->filename = "<unknown>";

int lineno = PyUnstable_InterpreterFrame_GetLine(pyframe);
if (lineno < 0) {
lineno = 0;
}
frame->lineno = (unsigned int)lineno;

PyObject *filename = filename = _PyFrame_GetCode(pyframe)->co_filename;
PyObject *filename = _PyFrame_GetCode(pyframe)->co_filename;
if (filename == NULL) {
#ifdef TRACE_DEBUG
tracemalloc_error("failed to get the filename of the code object");
Expand All @@ -245,18 +244,42 @@ tracemalloc_get_frame(_PyInterpreterFrame *pyframe, frame_t *frame)
return;
}

/* Copy filename to be held by tracemalloc. Subinterpreter frames may
disappear at any time, so instead of holding a reference, we must copy
the filename to ensure we don't lose it. */
Py_ssize_t filename_len;
const char *frame_filename = PyUnicode_AsUTF8AndSize(filename,
&filename_len);
if (frame_filename == NULL) {
#ifdef TRACE_DEBUG
tracemalloc_error("Could not convert filename to UTF8");
#endif
return;
}

/* intern the filename */
char *filename_copy;
_Py_hashtable_entry_t *entry;
entry = _Py_hashtable_get_entry(tracemalloc_filenames, filename);

entry = _Py_hashtable_get_entry(tracemalloc_filenames, frame_filename);
if (entry != NULL) {
filename = (PyObject *)entry->key;
filename_copy = (char *)entry->key;
}
else {
/* tracemalloc_filenames is responsible to keep a reference
to the filename */
if (_Py_hashtable_set(tracemalloc_filenames, Py_NewRef(filename),
/* tracemalloc_filenames is responsible for keeping a copy
of the filename */
filename_copy = (char*)raw_malloc(filename_len + 1);
if (filename_copy == NULL) {
#ifdef TRACE_DEBUG
tracemalloc_error("filename copy allocation failed");
#endif
return;
}

strcpy(filename_copy, frame_filename);
if (_Py_hashtable_set(tracemalloc_filenames, filename_copy,
NULL) < 0) {
Py_DECREF(filename);
raw_free(filename_copy);
#ifdef TRACE_DEBUG
tracemalloc_error("failed to intern the filename");
#endif
Expand All @@ -265,7 +288,7 @@ tracemalloc_get_frame(_PyInterpreterFrame *pyframe, frame_t *frame)
}

/* the tracemalloc_filenames table keeps a reference to the filename */
frame->filename = filename;
frame->filename = filename_copy;
}


Expand All @@ -281,7 +304,8 @@ traceback_hash(traceback_t *traceback)
x = 0x345678UL;
frame = traceback->frames;
while (--len >= 0) {
y = (Py_uhash_t)PyObject_Hash(frame->filename);
y = (Py_uhash_t)Py_HashBuffer(frame->filename,
strlen(frame->filename));
y ^= (Py_uhash_t)frame->lineno;
frame++;

Expand Down Expand Up @@ -698,19 +722,9 @@ tracemalloc_raw_realloc(void *ctx, void *ptr, size_t new_size)
}


static void
tracemalloc_clear_filename(void *value)
{
PyObject *filename = (PyObject *)value;
Py_DECREF(filename);
}


static void
tracemalloc_clear_traces_unlocked(void)
{
// Clearing tracemalloc_filenames requires the GIL to call Py_DECREF()
_Py_AssertHoldsTstate();

set_reentrant(1);

Expand All @@ -737,9 +751,9 @@ _PyTraceMalloc_Init(void)
return _PyStatus_NO_MEMORY();
}

tracemalloc_filenames = hashtable_new(hashtable_hash_pyobject,
tracemalloc_filenames = hashtable_new(hashtable_hash_filename,
hashtable_compare_unicode,
tracemalloc_clear_filename, NULL);
raw_free, NULL);

tracemalloc_tracebacks = hashtable_new(hashtable_hash_traceback,
hashtable_compare_traceback,
Expand All @@ -756,8 +770,7 @@ _PyTraceMalloc_Init(void)

tracemalloc_empty_traceback.nframe = 1;
tracemalloc_empty_traceback.total_nframe = 1;
/* borrowed reference */
tracemalloc_empty_traceback.frames[0].filename = &_Py_STR(anon_unknown);
tracemalloc_empty_traceback.frames[0].filename = "<unknown>";
tracemalloc_empty_traceback.frames[0].lineno = 0;
tracemalloc_empty_traceback.hash = traceback_hash(&tracemalloc_empty_traceback);

Expand Down Expand Up @@ -888,7 +901,8 @@ frame_to_pyobject(frame_t *frame)
return NULL;
}

PyTuple_SET_ITEM(frame_obj, 0, Py_NewRef(frame->filename));
PyObject *filename = PyUnicode_FromString(frame->filename);
PyTuple_SET_ITEM(frame_obj, 0, filename);

PyObject *lineno_obj = PyLong_FromUnsignedLong(frame->lineno);
if (lineno_obj == NULL) {
Expand Down Expand Up @@ -1144,7 +1158,7 @@ static void
_PyMem_DumpFrame(int fd, frame_t * frame)
{
PUTS(fd, " File \"");
_Py_DumpASCII(fd, frame->filename);
PUTS(fd, frame->filename);
PUTS(fd, "\", line ");
_Py_DumpDecimal(fd, frame->lineno);
PUTS(fd, "\n");
Expand Down
0