diff mbox

[RFA] Fix python gdbpy_breakpoint_object leak.

Message ID 20191112193501.21170-1-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Nov. 12, 2019, 7:35 p.m. UTC 
  valgrind reports a leak when a breakpoint is created then deleted:

==1313== 40 bytes in 1 blocks are definitely lost in loss record 1,115 of 8,596
==1313==    at 0x4835753: malloc (vg_replace_malloc.c:307)
==1313==    by 0x6E05BC: _PyObject_New (object.c:255)
==1313==    by 0x470E4B: gdbpy_breakpoint_created(breakpoint*) (py-breakpoint.c:1023)
==1313==    by 0x2946D9: operator() (std_function.h:687)
==1313==    by 0x2946D9: notify (observable.h:106)
==1313==    by 0x2946D9: install_breakpoint(int, std::unique_ptr<breakpoint, std::default_delete<breakpoint> >&&, int) (breakpoint.c:8136)
==1313==    by 0x295BCA: create_breakpoint_sal (breakpoint.c:8878)
==1313==    by 0x295BCA: create_breakpoints_sal (breakpoint.c:8919)
==1313==    by 0x295BCA: create_breakpoints_sal_default (breakpoint.c:13671)
...

The leak is due to a superfluous Py_INCREF when the python object
is allocated inside gdbpy_breakpoint_created, when the python object
is allocated locally: this object has already a refcount of 1, and
the only reference is the reference from the C breakpoint object.
The Py_INCREF is however needed when the python object was created from
python: the python object was stored in bppy_pending_object, and
gdbpy_breakpoint_created creates a new reference to this object.

Solve the leak by calling 'Py_INCREF (newbp);' only in the bppy_pending_object
case.

Regression tested on debian/amd64 natively and under valgrind on centos/amd64.
Before the patch, 795 tests have a definite leak.
After the patch, 197 have a definite leak.

Thanks to Tom, that helped on irc with the python refcount logic.

gdb/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* python/py-finishbreakpoint.c (gdbpy_breakpoint_created):
	only call Py_INCREF (newbp) in the bppy_pending_object case.
---
 gdb/python/py-breakpoint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Tromey Nov. 12, 2019, 10:27 p.m. UTC | #1 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> gdb/ChangeLog
Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* python/py-finishbreakpoint.c (gdbpy_breakpoint_created):
Philippe> 	only call Py_INCREF (newbp) in the bppy_pending_object case.

Thank you.  This is ok.

Tom
diff mbox

Patch

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 65cb29fdca..4170737416 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1017,6 +1017,7 @@  gdbpy_breakpoint_created (struct breakpoint *bp)
   if (bppy_pending_object)
     {
       newbp = bppy_pending_object;
+      Py_INCREF (newbp);
       bppy_pending_object = NULL;
     }
   else
@@ -1027,7 +1028,6 @@  gdbpy_breakpoint_created (struct breakpoint *bp)
       newbp->bp = bp;
       newbp->bp->py_bp_object = newbp;
       newbp->is_finish_bp = 0;
-      Py_INCREF (newbp);
       ++bppy_live;
     }
   else