[lldb-dev] Too many ModuleSP references

[Greg Clayton via lldb-dev]

> On Feb 21, 2017, at 9:36 AM, Jim Ingham via lldb-dev <lldb-dev at lists.llvm.org> wrote:
> 
> Thanks for digging into this.
> 
> Maybe a better option would be to have IRExecutionUnit hold a SymbolContextScope, rather than a SymbolContext.  We started out with SymbolContext's because they were convenient but they hold too many things alive so when we run into SymbolContext's we've been converting them to the SymbolContextScope's when possible.  I can't see any need for the IRExecutionUnit to hold the module alive, so long as it handles failure to get the SymbolContext gracefully, which shouldn't be hard.

SymbolContextScope doesn't hold ownership as it is just a pointer. This pointer would go bad if the module goes away and it would crash when you tried to use it. See my other email for another approach with SymbolContextRef.

Greg

> 
> Jim
> 
>> On Feb 21, 2017, at 9:03 AM, Pavel Labath via lldb-dev <lldb-dev at lists.llvm.org> wrote:
>> 
>> Hello all,
>> 
>> I've been debugging the newly added TestStepOverBreakpoint.py, which
>> has been failing on windows, for a very prosaic reason: after the test
>> completes, we are unable to run "make clean" because lldb still holds
>> the file open.
>> 
>> After some debugging, I've found that this happens because the test
>> case stores the SBBreakpoint object in a member variable of the python
>> test case class. The breakpoint class ends up transitively holding a
>> reference to the main executable module, which prevents the module
>> from being garbage-collected when the target is destroyed.
>> 
>> For reference, the full ownership chain is something like:
>> StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
>> BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
>> SymbolContext => Module.
>> 
>> To get the test working, we need to break this chain somewhere. A
>> couple of places I see are:
>> - BreakpointLocation: Remove the compiled expression reference when
>> the target is destroyed (AFAICS, it is used as a cache to avoid
>> recomputing the expression every time. It can be theoretically
>> recomputed if needed, but that shouldn't be necessary as the target is
>> destroyed anyway)
>> 
>> - SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
>> object. When the target is destroyed, the SBBreakpoint object becomes
>> invalid (One doesn't cannot do anything useful with the breakpoint
>> once the target has been deleted anyway).
>> 
>> - StepOverBreakpointTestCase: Have the test not store the breakpoints
>> in the test case object. Basically, declare that this is not a bug,
>> and it's the users responsibility to clean up necessary objects.
>> 
>> Any thoughts on what is the appropriate solution here?
>> 
>> cheers,
>> pavel
>> _______________________________________________
>> lldb-dev mailing list
>> lldb-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev