[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

[Cameron via Phabricator via lldb-commits]

cameron314 created this revision.
cameron314 added reviewers: labath, jingham.
cameron314 added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, aprantl.

This fixes a memory leak with several synthetic children front-ends.

The lifetime of a ValueObject and all its derivative ValueObjects (children, clones, etc.) is managed by a ClusterManager. These objects are only destroyed when every shared pointer to any of the managed objects in the cluster is destroyed. This means that no object in the cluster can store a shared pointer to another object in the cluster without creating a memory leak of the entire cluster. However, some of the synthetic children front-end implementations do exactly this; this patch fixes that.

The memory leak is actually much more important than just the lost memory, since it prevents the file handle to the binary (.elf in my case) from being closed due to references to the debug info from the ValueObject hierarchy. This means a build system is blocked from rebuilding the binary even after LLDB has disconnected.

Note that most of the existing synthetic children front-ends correctly avoid this buggy shared-pointer pattern with explicit comments about this exact type of memory leak. For example, in the NSErrorSyntheticFrontEnd:

  // the child here can be "real" (i.e. an actual child of the root) or
  // synthetized from raw memory if the former, I need to store a plain pointer
  // to it - or else a loop of references will cause this entire hierarchy of
  // values to leak if the latter, then I need to store a SharedPointer to it -
  // so that it only goes away when everyone else in the cluster goes away oh
  // joy!
  ValueObject *m_child_ptr;
  ValueObjectSP m_child_sp;

and the LibCxxMapIteratorSyntheticFrontEnd:

  // this must be a ValueObject* because it is a child of the ValueObject we
  // are producing children for it if were a ValueObjectSP, we would end up
  // with a loop (iterator -> synthetic -> child -> parent == iterator) and
  // that would in turn leak memory by never allowing the ValueObjects to die
  // and free their memory

Apparently I'm not the first to lose a few hours debugging this :-)


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68641

Files:
  source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
  source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68641.223650.patch
Type: text/x-patch
Size: 7534 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20191008/5b4b0cf0/attachment.bin>