[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Sure, if somebody has the time fixing this to use weak pointers would be great.

But that doesn't seem like the real issue to me.

When a breakpoint gets removed from the Target BreakpointList(s), regardless of who else is holding onto it, it needs to get its breakpoint sites removed (since the Target can no longer reason about them they will look like unrecognized traps.)  We can't rely on Destructors to do this as long as we are handing out BreakpointSP's.  So you still need an explicit operation to clear the breakpoint sites.

I can't think of any compelling use for preserving a Breakpoint once it is removed from the Target.  So I'm pretty convinced we should go to the model where removing a Breakpoint from the Target's BreakpointList immediately deletes the Breakpoint.  Then we can fix this dependency so the destructors do the right thing.  Note, for the sake of making an important step less indirect, I would vote to also have the Breakpoint destructor call ClearBreakpointSites().

This is a more intrusive change, since it means either that everybody who is now holding BreakpointSP's should switch over to holding BreakpointID's or BreakpointWP's.  But that models more closely what should happen with breakpoints.


https://reviews.llvm.org/D45554