Delete Breakpoint objects, fixing memory leak

runtime/vm/debugger.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/debugger.h"
 
 #include "include/dart_api.h"
 
 #include "platform/address_sanitizer.h"
 
@@ skipping 178 matching lines @@
   // Compute column number lazily since it causes scanning of the script.
   if (column_number_ < 0) {
     const Script& script = Script::Handle(this->script());
     script.GetTokenLocation(token_pos_, &line_number_, &column_number_);
   }
   return column_number_;
 }
 
 
 void Breakpoint::set_bpt_location(BreakpointLocation* new_bpt_location) {
-  ASSERT(bpt_location_->IsLatent());  // Only reason to move.
+  // Only latent breakpoints can be moved.
+  ASSERT((new_bpt_location == NULL) || bpt_location_->IsLatent());
   bpt_location_ = new_bpt_location;
 }
 
 
 void Breakpoint::VisitObjectPointers(ObjectPointerVisitor* visitor) {
   visitor->VisitPointer(reinterpret_cast<RawObject**>(&closure_));
 }
 
 
 void BreakpointLocation::VisitObjectPointers(ObjectPointerVisitor* visitor) {
@@ skipping 1136 matching lines @@
       resume_action_(kContinue),
       resume_frame_index_(-1),
       post_deopt_frame_index_(-1),
       ignore_breakpoints_(false),
       pause_event_(NULL),
       obj_cache_(NULL),
       stack_trace_(NULL),
       async_causal_stack_trace_(NULL),
       stepping_fp_(0),
       skip_next_step_(false),
+      needs_breakpoint_cleanup_(false),
       synthetic_async_breakpoint_(NULL),
       exc_pause_info_(kNoPauseOnExceptions) {}
 
 
 Debugger::~Debugger() {
   isolate_id_ = ILLEGAL_ISOLATE_ID;
   ASSERT(!IsPaused());
   ASSERT(latent_locations_ == NULL);
   ASSERT(breakpoint_locations_ == NULL);
   ASSERT(code_breakpoints_ == NULL);
@@ skipping 1413 matching lines @@
            error.IsUnhandledException());
 
     // Only send a resume event when the isolate is not unwinding.
     if (!error.IsUnwindError()) {
       ServiceEvent resume_event(event->isolate(), ServiceEvent::kResume);
       resume_event.set_top_frame(event->top_frame());
       Service::HandleEvent(&resume_event);
     }
   }
 
+  if (needs_breakpoint_cleanup_) {
+    RemoveUnlinkedCodeBreakpoints();
+  }
   pause_event_ = NULL;
   obj_cache_ = NULL;  // Zone allocated
 }
 
 
 void Debugger::EnterSingleStepMode() {
   stepping_fp_ = 0;
   DeoptimizeWorld();
   isolate_->set_single_step(true);
 }
@@ skipping 470 matching lines @@
     // We issue a step over to resume at the point after the await statement.
     SetResumeAction(kStepOver);
     // When we single step from a user breakpoint, our next stepping
     // point will be at the exact same pc.  Skip it.
     HandleSteppingRequest(stack_trace_, true /* skip next step */);
     ClearCachedStackTraces();
     return Error::null();
   }
 
   if (FLAG_verbose_debug) {
-    OS::Print(">>> hit %s breakpoint at %s:%" Pd
-              " "
-              "(token %s) (address %#" Px ")\n",
-              cbpt->IsInternal() ? "internal" : "user",
+    OS::Print(">>> hit breakpoint at %s:%" Pd " (token %s) (address %#" Px
+              ")\n",
               String::Handle(cbpt->SourceUrl()).ToCString(), cbpt->LineNumber(),
               cbpt->token_pos().ToCString(), top_frame->pc());
   }
 
   CacheStackTraces(stack_trace, CollectAsyncCausalStackTrace());
   SignalPausedEvent(top_frame, bpt_hit);
   // When we single step from a user breakpoint, our next stepping
   // point will be at the exact same pc.  Skip it.
   HandleSteppingRequest(stack_trace_, true /* skip next step */);
   ClearCachedStackTraces();
-  if (cbpt->IsInternal()) {
-    RemoveInternalBreakpoints();
-  }
 
   // If any error occurred while in the debug message loop, return it here.
   const Error& error = Error::Handle(Thread::Current()->sticky_error());
   Thread::Current()->clear_sticky_error();
   return error.raw();
 }
 
 
 Breakpoint* Debugger::FindHitBreakpoint(BreakpointLocation* location,
                                         ActivationFrame* top_frame) {
@@ skipping 356 matching lines @@
     Breakpoint* prev_bpt = NULL;
     Breakpoint* curr_bpt = curr_loc->breakpoints();
     while (curr_bpt != NULL) {
       if (curr_bpt->id() == bp_id) {
         if (prev_bpt == NULL) {
           curr_loc->set_breakpoints(curr_bpt->next());
         } else {
           prev_bpt->set_next(curr_bpt->next());
         }
 
+        // Send event to client before the breakpoint's fields are
+        // poisoned and deleted.
         SendBreakpointEvent(ServiceEvent::kBreakpointRemoved, curr_bpt);
 
-        // Remove references from the current debugger pause event.
+        curr_bpt->set_next(NULL);
+        curr_bpt->set_bpt_location(NULL);
+        // Remove possible references to the breakpoint.
         if (pause_event_ != NULL && pause_event_->breakpoint() == curr_bpt) {
           pause_event_->set_breakpoint(NULL);
         }
-        break;
+        if (synthetic_async_breakpoint_ == curr_bpt) {
+          synthetic_async_breakpoint_ = NULL;
+        }
+        delete curr_bpt;
+        curr_bpt = NULL;
+
+        // Delete the breakpoint location object if there are no more
+        // breakpoints at that location.
+        if (curr_loc->breakpoints() == NULL) {
+          if (prev_loc == NULL) {
+            breakpoint_locations_ = curr_loc->next();
+          } else {
+            prev_loc->set_next(curr_loc->next());
+          }
+
+          // Remove references from code breakpoints to this breakpoint
+          // location and disable them.
+          UnlinkCodeBreakpoints(curr_loc);
+          BreakpointLocation* next_loc = curr_loc->next();
+          delete curr_loc;
+          curr_loc = next_loc;
+        }
+
+        // The code breakpoints will be deleted when the VM resumes
+        // after the pause event.
+        return;
       }
 
       prev_bpt = curr_bpt;
       curr_bpt = curr_bpt->next();
     }
-
-    if (curr_loc->breakpoints() == NULL) {
-      if (prev_loc == NULL) {
-        breakpoint_locations_ = curr_loc->next();
-      } else {
-        prev_loc->set_next(curr_loc->next());
-      }
-
-      // Remove references from code breakpoints to this source breakpoint,
-      // and disable the code breakpoints.
-      UnlinkCodeBreakpoints(curr_loc);
-      BreakpointLocation* next_loc = curr_loc->next();
-      delete curr_loc;
-      curr_loc = next_loc;
-    } else {
-      prev_loc = curr_loc;
-      curr_loc = curr_loc->next();
-    }
+    prev_loc = curr_loc;
+    curr_loc = curr_loc->next();
   }
-  // bpt is not a registered breakpoint, nothing to do.
+  // breakpoint with bp_id does not exist, nothing to do.
 }
 
 
-// Turn code breakpoints associated with the given source breakpoint into
-// internal breakpoints. They will later be deleted when control
-// returns from the user-defined breakpoint callback. Also, disable the
-// breakpoint so it no longer fires if it should be hit before it gets
-// deleted.
+// Unlink code breakpoints from the the given breakpoint location.
+// They will later be deleted when control returns from the pause event
+// callback. Also, disable the breakpoint so it no longer fires if it
+// should be hit before it gets deleted.
 void Debugger::UnlinkCodeBreakpoints(BreakpointLocation* bpt_location) {
   ASSERT(bpt_location != NULL);
   CodeBreakpoint* curr_bpt = code_breakpoints_;
   while (curr_bpt != NULL) {
     if (curr_bpt->bpt_location() == bpt_location) {
       curr_bpt->Disable();
       curr_bpt->set_bpt_location(NULL);
+      needs_breakpoint_cleanup_ = true;
     }
     curr_bpt = curr_bpt->next();
   }
 }
 
 
-// Remove and delete internal breakpoints, i.e. breakpoints that
-// are not associated with a source breakpoint.
-void Debugger::RemoveInternalBreakpoints() {
+// Remove and delete unlinked code breakpoints, i.e. breakpoints that
+// are not associated with a breakpoint location.
+void Debugger::RemoveUnlinkedCodeBreakpoints() {
   CodeBreakpoint* prev_bpt = NULL;
   CodeBreakpoint* curr_bpt = code_breakpoints_;
   while (curr_bpt != NULL) {
     if (curr_bpt->bpt_location() == NULL) {
       if (prev_bpt == NULL) {
         code_breakpoints_ = code_breakpoints_->next();
       } else {
         prev_bpt->set_next(curr_bpt->next());
       }
       CodeBreakpoint* temp_bpt = curr_bpt;
       curr_bpt = curr_bpt->next();
       temp_bpt->Disable();
       delete temp_bpt;
     } else {
       prev_bpt = curr_bpt;
       curr_bpt = curr_bpt->next();
     }
   }
+  needs_breakpoint_cleanup_ = false;
 }
 
 
 BreakpointLocation* Debugger::GetBreakpointLocation(const Script& script,
                                                     TokenPosition token_pos,
                                                     intptr_t requested_column) {
   BreakpointLocation* bpt = breakpoint_locations_;
   while (bpt != NULL) {
     if ((bpt->script_ == script.raw()) && (bpt->token_pos_ == token_pos) &&
         (bpt->requested_column_number_ == requested_column)) {
@@ skipping 51 matching lines @@
 
 void Debugger::RegisterCodeBreakpoint(CodeBreakpoint* bpt) {
   ASSERT(bpt->next() == NULL);
   bpt->set_next(code_breakpoints_);
   code_breakpoints_ = bpt;
 }
 
 #endif  // !PRODUCT
 
 }  // namespace dart