[Lldb-commits] [lldb] [lldb][test] fix sporadic failures with NumberAllocatedModules assert (PR #176884)

[via lldb-commits]

jimingham wrote:

> @Michael137, @jimingham to investigate the root cause of the memory leak, I set up a shared_ptr ownership tracker to identify what holds references to the shared_ptrs on modules that were not destroyed and lead to the assertion failure.
> 
> The results indicate that the underlying cause is a shared_ptr cycle rather than the race condition during the destroy phase I had previously suspected. Specifically, `ThreadPlanCallFunction` holds a shared_ptr to `StopInfo` (`m_real_stop_info_sp`), while `StopInfoThreadPlan` holds a shared_ptr to `ThreadPlan` (`m_plan_sp`). In certain scenarios, instances of these two classes end up referencing each other, forming a cycle that prevents destruction and causes additional memory leaks, in particular, module leakage, as shown in the diagram:
> 
> <img alt="shared_ptr_leakage" width="1378" height="743" src="https://private-user-images.githubusercontent.com/175604663/564214981-8034e35e-d081-4526-9f29-76ccc3066a03.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzM2Nzg1MDcsIm5iZiI6MTc3MzY3ODIwNywicGF0aCI6Ii8xNzU2MDQ2NjMvNTY0MjE0OTgxLTgwMzRlMzVlLWQwODEtNDUyNi05ZjI5LTc2Y2NjMzA2NmEwMy5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjYwMzE2JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI2MDMxNlQxNjIzMjdaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0xZDE5N2VjMWNkMjNiZDQxZTE1N2EzZTUzM2I5ZGMxNzFhM2NhNmI0MGM1ZDVkOWU1NDBjNGEzNWNlMWFjMmY2JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.ecAhRFakzCX4JrXoNy4nXTC8G1cZE3QNZ1sKTo4D2wo">
> The sporadic nature of the phenomenon and the difficulty in reproducing it can be explained by the narrow conditions required to trigger the cycle. I mean to my knowledge, `ThreadPlanCallFunction` exists on the thread plan stack only during expression evaluation, so `StopInfoThreadPlan` can acquire a reference to it within that period, therefore I suspect the cycle forms when execution stops right in the middle of the expression evaluation, which seems as a fairly narrow time window.
> 
> A straightforward fix would be to replace `m_real_stop_info_sp` with a weak_ptr, which the ownership tracker logs confirm eliminates the leakage. However, this feels more like a workaround than a proper solution. The right approach is to understand why the cycle forms in the first place. It seems to me that the name `m_real_stop_info_sp` suggests it is intended to hold a raw stop info (i.e., one originating directly from lldb-server), which makes the presence of a `StopInfoThreadPlan` there semantically suspect.
> 
> Besides, I wonder whether addressing this issue might fix the NumberAllocatedModules assertion failure on Windows, given that it is currently disabled on that platform (`tearDown` hook in `lldbtest.py`).

Did you see how m_real_stop_info_sp gets set to the ThreadPlan stop info?  The m_real_stop_info_sp always gets set by calling ThreadPlan::GetPrivateStopInfo -> Thread::GetPrivateStopInfo() -> ProcessGDBRemote::CalculateStopInfoForThread, and ProcessGDBRemote doesn't know about thread plans, so it never have returned a StopInfoThreadPlan.

I'm also curious how ThreadPlanCallFunction pushed anything but a ThreadPlanRunToAddress, that is the only thread plan that it needs to use (and the only one it does push according to the code.).  ThreadPlanRunToAddress only holds a vector of lldb::addr_t's so it doesn't have a reference to modules.

https://github.com/llvm/llvm-project/pull/176884