[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Jan 17 02:58:52 PST 2022
labath added a comment.
Unifying the two paths by making turning the other reference into an owned one is a step in the right direction, but it is not enough, as StructuredPythonObject expects a *borrowed* reference. So, instead of fixing things, you've made both code paths leak. :)
I've created D117462 <https://reviews.llvm.org/D117462>, which should make reasoning about ownership much easier. StructuredPythonObject and all of the LLDBSwigPython functions now tracks ownership automatically, and one only needs to be careful when converting to/from a void *. Ideally those would be removed too, but that would require a bigger refactor. If you rebase this patch on top of that, you should be able to transfer ownership correctly.
While working on the patch I also realized that pretty much all of our StructuredPythonObject interfaces were leaking. So, in a way, you were very unfortunate to hit a use after free here (like, it it must have taken multiple decrefs on borrowed references (one for each thread, I guess) to undo all of those leaks). OTOH, we were also fortunate that you've found this, as we can now improve the interfaces and fix those leaks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117071/new/
https://reviews.llvm.org/D117071
More information about the lldb-commits
mailing list