fix: prevent code objects from leaking #1924

nedbat · Feb 6, 2025 · f85d9b7 · f85d9b7
1 parent ae8d3b9
commit f85d9b7
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 11 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -23,11 +23,16 @@ upgrading your version of coverage.py.
 Unreleased
 ----------
 
+- Fix: a memory leak in CTracer has been fixed.  The details are in `issue
+  1924`_ and `pytest-dev 676`_.  This should reduce the memory footprint for
+  everyone even if it hadn't caused a problem before.
+
 - We now ship a py3-none-any.whl wheel file.  Thanks, `Russell Keith-Magee
   <pull 1914_>`_.
 
 .. _pull 1914: https://github.com/nedbat/coveragepy/pull/1914
-
+.. _issue 1924: https://github.com/nedbat/coveragepy/issues/1924
+.. _pytest-dev 676: https://github.com/pytest-dev/pytest-cov/issues/676
 
 .. start-releases
 

diff --git a/coverage/ctracer/tracer.c b/coverage/ctracer/tracer.c
@@ -288,6 +288,17 @@ CTracer_set_pdata_stack(CTracer *self)
     return ret;
 }
 
+// Thanks for the idea, memray!
+inline PyCodeObject*
+MyFrame_BorrowCode(PyFrameObject* frame)
+{
+    // Return a borrowed reference.
+    PyCodeObject* pCode = PyFrame_GetCode(frame);
+    assert(Py_REFCNT(pCode) >= 2);
+    Py_DECREF(pCode);
+    return pCode;
+}
+
 /*
  * Parts of the trace function.
  */
@@ -359,7 +370,7 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
     }
 
     /* Check if we should trace this line. */
-    filename = PyFrame_GetCode(frame)->co_filename;
+    filename = MyFrame_BorrowCode(frame)->co_filename;
     disposition = PyDict_GetItem(self->should_trace_cache, filename);
     if (disposition == NULL) {
         if (PyErr_Occurred()) {
@@ -554,15 +565,15 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
      * The current opcode is guaranteed to be RESUME. The argument
      * determines what kind of resume it is.
      */
-    pCode = MyCode_GetCode(PyFrame_GetCode(frame));
+    pCode = MyCode_GetCode(MyFrame_BorrowCode(frame));
     real_call = (PyBytes_AS_STRING(pCode)[MyFrame_GetLasti(frame) + 1] == 0);
 #else
     // f_lasti is -1 for a true call, and a real byte offset for a generator re-entry.
     real_call = (MyFrame_GetLasti(frame) < 0);
 #endif
 
     if (real_call) {
-        self->pcur_entry->last_line = -PyFrame_GetCode(frame)->co_firstlineno;
+        self->pcur_entry->last_line = -MyFrame_BorrowCode(frame)->co_firstlineno;
     }
     else {
         self->pcur_entry->last_line = PyFrame_GetLineNumber(frame);
@@ -649,7 +660,7 @@ CTracer_handle_line(CTracer *self, PyFrameObject *frame)
 
     STATS( self->stats.lines++; )
     if (self->pdata_stack->depth >= 0) {
-        SHOWLOG(PyFrame_GetLineNumber(frame), PyFrame_GetCode(frame)->co_filename, "line");
+        SHOWLOG(PyFrame_GetLineNumber(frame), MyFrame_BorrowCode(frame)->co_filename, "line");
         if (self->pcur_entry->file_data) {
             int lineno_from = -1;
             int lineno_to = -1;
@@ -727,7 +738,7 @@ CTracer_handle_return(CTracer *self, PyFrameObject *frame)
         self->pcur_entry = &self->pdata_stack->stack[self->pdata_stack->depth];
         if (self->tracing_arcs && self->pcur_entry->file_data) {
             BOOL real_return = FALSE;
-            pCode = MyCode_GetCode(PyFrame_GetCode(frame));
+            pCode = MyCode_GetCode(MyFrame_BorrowCode(frame));
             int lasti = MyFrame_GetLasti(frame);
             Py_ssize_t code_size = PyBytes_GET_SIZE(pCode);
             unsigned char * code_bytes = (unsigned char *)PyBytes_AS_STRING(pCode);
@@ -759,7 +770,7 @@ CTracer_handle_return(CTracer *self, PyFrameObject *frame)
             real_return = !(is_yield || is_yield_from);
 #endif
             if (real_return) {
-                int first = PyFrame_GetCode(frame)->co_firstlineno;
+                int first = MyFrame_BorrowCode(frame)->co_firstlineno;
                 if (CTracer_record_pair(self, self->pcur_entry->last_line, -first) < 0) {
                     goto error;
                 }
@@ -782,7 +793,7 @@ CTracer_handle_return(CTracer *self, PyFrameObject *frame)
         }
 
         /* Pop the stack. */
-        SHOWLOG(PyFrame_GetLineNumber(frame), PyFrame_GetCode(frame)->co_filename, "return");
+        SHOWLOG(PyFrame_GetLineNumber(frame), MyFrame_BorrowCode(frame)->co_filename, "return");
         self->pdata_stack->depth--;
         self->pcur_entry = &self->pdata_stack->stack[self->pdata_stack->depth];
     }
@@ -824,13 +835,13 @@ CTracer_trace(CTracer *self, PyFrameObject *frame, int what, PyObject *arg_unuse
     if (what <= (int)(sizeof(what_sym)/sizeof(const char *))) {
         w = what_sym[what];
     }
-    ascii = PyUnicode_AsASCIIString(PyFrame_GetCode(frame)->co_filename);
+    ascii = PyUnicode_AsASCIIString(MyFrame_BorrowCode(frame)->co_filename);
     printf("%x trace: f:%x %s @ %s %d\n", (int)self, (int)frame, what_sym[what], PyBytes_AS_STRING(ascii), PyFrame_GetLineNumber(frame));
     Py_DECREF(ascii);
     #endif
 
     #if TRACE_LOG
-    ascii = PyUnicode_AsASCIIString(PyFrame_GetCode(frame)->co_filename);
+    ascii = PyUnicode_AsASCIIString(MyFrame_BorrowCode(frame)->co_filename);
     if (strstr(PyBytes_AS_STRING(ascii), start_file) && PyFrame_GetLineNumber(frame) == start_line) {
         logging = TRUE;
     }
@@ -926,7 +937,7 @@ CTracer_call(CTracer *self, PyObject *args, PyObject *kwds)
     }
 
     #if WHAT_LOG
-    ascii = PyUnicode_AsASCIIString(PyFrame_GetCode(frame)->co_filename);
+    ascii = PyUnicode_AsASCIIString(MyFrame_BorrowCode(frame)->co_filename);
     printf("pytrace: %s @ %s %d\n", what_sym[what], PyBytes_AS_STRING(ascii), PyFrame_GetLineNumber(frame));
     Py_DECREF(ascii);
     #endif

diff --git a/tests/test_oddball.py b/tests/test_oddball.py
@@ -206,6 +206,21 @@ def once(x):                                        # line 301
         if fails > 8:
             pytest.fail("RAM grew by %d" % (ram_growth))      # pragma: only failure
 
+    @pytest.mark.skipif(not testenv.C_TRACER, reason="Only the C tracer has refcounting issues")
+    # In fact, sysmon explicitly holds onto all code objects,
+    # so this will definitely fail with sysmon.
+    def test_eval_codeobject_leak(self) -> None:
+        # https://github.com/nedbat/coveragepy/issues/1924
+        code = """\
+            for i in range(100_000):
+                r = eval("'a' + '1'")
+                assert r == 'a1'
+            """
+        ram_0 = osinfo.process_ram()
+        self.check_coverage(code, [1, 2, 3], "")
+        ram_growth = osinfo.process_ram() - ram_0
+        assert ram_growth < 2_000 * 1024
+
 
 class MemoryFumblingTest(CoverageTest):
     """Test that we properly manage the None refcount."""