gdb/python: fix reference leak in gdb.BreakpointLocation.thread_groups
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
Commit Message
While reviewing another patch which uses PyList_Append I took a look
at our other uses of PyList_Append in GDB. I spotted something odd
about the use in bplocpy_get_thread_groups.
We do:
gdbpy_ref<> num = gdb_py_object_from_ulongest (inf->num);
At which point `num` will own a reference to the `int` object. But
when we add the object to the result list we do:
if (PyList_Append (list.get (), num.release ()) != 0)
return nullptr;
By calling `release` we pass ownership of the reference to
PyList_Append, however, PyList_Append acquires its own reference, it
doesn't take ownership of an existing reference.
The consequence of this is that we leak the reference held in `num`.
This mostly isn't a problem though. For small (< 257) integers Python
keeps a single instance of each and just hands out new references. By
leaking the references, these small integers will not be cleaned up as
the Python interpreter shuts down, but that is only done when GDB
exits, so hardly a disaster. As we're dealing with GDB's internal
inferior number here, unless the user has 257+ inferiors, we'll not
actually be leaking memory.
Still, lets do things right. Switch to using `num.get ()`. Now when
`num` goes out of scope it will decrement the reference count as
needed.
---
gdb/python/py-breakpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
base-commit: 82eff6743b77908a502b4cf9030acc93caf69e74
Comments
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> Still, lets do things right. Switch to using `num.get ()`. Now when
Andrew> `num` goes out of scope it will decrement the reference count as
Andrew> needed.
Thanks for finding this.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
Tom Tromey <tom@tromey.com> writes:
>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> Still, lets do things right. Switch to using `num.get ()`. Now when
> Andrew> `num` goes out of scope it will decrement the reference count as
> Andrew> needed.
>
> Thanks for finding this.
> Approved-By: Tom Tromey <tom@tromey.com>
Pushed.
Thanks,
Andrew
@@ -1690,7 +1690,7 @@ bplocpy_get_thread_groups (PyObject *py_self, void *closure)
gdbpy_ref<> num = gdb_py_object_from_ulongest (inf->num);
if (num == nullptr)
return nullptr;
- if (PyList_Append (list.get (), num.release ()) != 0)
+ if (PyList_Append (list.get (), num.get ()) != 0)
return nullptr;
}
}