[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.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list