gh-134604: Fix tracemalloc crash with subinterpreters

[emmatyping]

This is an attempt at resolving a crash when mixing tracemalloc and sub-interpreters I came across recently. My investigation is recorded in the issue #134604 (comment)

This PR makes a few changes:

• modifies tracemalloc to copy the filename of a frame instead of referencing it
• delays sub-interpreter finalization until after tracemalloc finalizes (optional, but probably a good idea)
• fixes a typo/duplicated assignment on a line in the tracemalloc_get_frame function
• adds tests to test__interpreters to ensure that this functions correctly.

Should this be backported to 3.13? I suppose subinterpreters can be used there through C APIs, so it might be good to fix this there too.

• Issue: Creating a subinterpreter when Python is run with -X tracemalloc leads to segfault #134604

[ZeroIntensity]

I think there's more to be fixed than just the filename. Doesn't tracemalloc have mutable state accessible by all interpreters?

[emmatyping]

I think there's more to be fixed than just the filename. Doesn't tracemalloc have mutable state accessible by all interpreters?

If you mean the allocator/tracemalloc runtime state, that is guarded by a lock.

[ZeroIntensity]

Right, but some of the state that it stores looks like it could be problematic across interpreters. I'll investigate.

[emmatyping]

Yeah the following test hangs:

deftest_tracemalloc_multiple_subinterpreters(self): num_threads=8threads= [] defrun_interp(): interpid=_interpreters.create() _interpreters.run_string( interpid, "def foo(a, b): return a + b; foo(1, 5)") _interpreters.destroy(interpid) foriinrange(num_threads): thread=threading.Thread(target=run_interp) threads.append(thread) withthreading_helper.start_threads(threads): passstats=tracemalloc.take_snapshot().statistics("filename") count=sum(trace.traceback._frames[0][0] =="<string>"fortraceinstats) self.assertEqual(count, num_threads)

[vstinner]

modifies tracemalloc to copy the filename of a frame instead of referencing it

tracemalloc_filenames stores strong references to Unicode strings. What is the problem with sub-interpreters? I would prefer to keep Unicode objects. For example, _Py_DumpASCII(fd, filename) has a different behavior than PUTS(fd, filename).

[vstinner]

Would it be possible to modify memory allocation hooks (malloc, calloc, realloc, free) to detect which interpreter is used, and do nothing if it's called from an interpreter different than the one which called tracemalloc.start()? Limit tracemalloc to a single interpreter at the same time.

The problem is that tracemalloc_raw_malloc(), tracemalloc_raw_calloc(), tracemalloc_raw_realloc() and tracemalloc_free() are called without the GIL. The first 3 functions call PyGILState_Ensure() / PyGILState_Release() to acquire the GIL, but these functions are known to not work with sub-interpreters... We would need PEP 788 API here.

[emmatyping]

Would it be possible to modify memory allocation hooks (malloc, calloc, realloc, free) to detect which interpreter is used, and do nothing if it's called from an interpreter different than the one which called tracemalloc.start()? Limit tracemalloc to a single interpreter at the same time.

Yeah I think something like that could be done. I think it might be better to shard the state it tracks to be per-interpreter however. I expect users may wish to call tracemalloc.start() within a sub-interpreter. Also what would the behavior of -X tracemalloc be if it only traced one interpreter? Tracing the main one I presume?

[emmatyping]

I'm going to go ahead and close this PR since the solution here is not sufficient. I'll think more about a proper solution and maybe we can figure that out in the issue.

[vstinner]

Thanks for trying, it's a hard problem.