[Lldb-commits] [PATCH] D117071: [lldb/Plugins] Add support of multiple ScriptedThreads in a ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D117071#3238816 <https://reviews.llvm.org/D117071#3238816>, @JDevlieghere wrote:

> Never thought I'd ask someone to merge two patches, but I think it might make reviewing easier if you merge D117139 <https://reviews.llvm.org/D117139> into this patch. :-)

Done ^^

> If I understand the patch correctly, you're getting the underlying `PyObject` out of the `PythonDateObject` when passing it to the scripted thread interface. I assume that works because the objects are retained by being stored in a dict on the Python side (referring to `threads` in ScriptedProcess, from D117068 <https://reviews.llvm.org/D117068>). Why can't we pass the PythonObject around instead? That seems simpler but more importantly would guarantee the underlying object remains alive (even if it weren't stored on the Python side)  by keeping the ref-count incremented.

Right. Sure, it would be easier to pass the PythonObject around but the Scripted(Thread)Interface tries to be as language-agnostic as possible, that's why we use `StructuredData::Generic` instead. I think it should remain this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117071/new/

https://reviews.llvm.org/D117071