This is an archive of the discontinued LLVM Phabricator instance.

[LLDB][NFC] Fix memory/resource leak in FifoFiles
AbandonedPublic

Authored by fixathon on Jul 29 2022, 3:08 PM.

Details

Summary

The original code is creating a future object on the heap, but does not delete it in one of the return paths causing a leak.
Because the lifetime of the future object is the local scope of its containing function, there's no need for heap-based allocation in the first place.
This diff fixes the memory leak by moving the future object allocation to stack-based RAII. There's no change to the functionality or style of the code.

Diff Detail

Event Timeline

fixathon created this revision.Jul 29 2022, 3:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2022, 3:08 PM
fixathon requested review of this revision.Jul 29 2022, 3:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2022, 3:08 PM
clayborg accepted this revision.Jul 29 2022, 3:16 PM
This revision is now accepted and ready to land.Jul 29 2022, 3:16 PM
clayborg requested changes to this revision.Jul 29 2022, 3:23 PM
clayborg added inline comments.
lldb/tools/lldb-vscode/FifoFiles.cpp
53–54

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?

This revision now requires changes to proceed.Jul 29 2022, 3:23 PM
fixathon added inline comments.Jul 29 2022, 3:55 PM
lldb/tools/lldb-vscode/FifoFiles.cpp
53–54

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.

fixathon abandoned this revision.Jul 29 2022, 4:21 PM