[Lldb-commits] [PATCH] D130802: [LLDB][NFC] Fix memory/resource leak in FifoFiles

[Slava Gurevich via Phabricator via lldb-commits]

fixathon added inline comments.


================
Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:53-54
 Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
   // We use a pointer for this future, because otherwise its normal destructor
   // would wait for the getline to end, rendering the timeout useless.
   Optional<std::string> line;
----------------
clayborg wrote:
> This comment seems to indicate we need to use a pointer. Seems like this should either be rewritten to not try to use a future with a timeout or left as is?
Good catch. Looks like the original code aims to intentionally orphan the 'future' object if it's blocked (possibly infinitely) in its destructor waiting for the executing thread to rejoin. This could happen if the workload uses blocking I/O calls like we have here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130802