Prevent calls to deleted breakpoint listeners

Matt Loring · Matt Loring · commit a26278e7028d · 2016-06-10T16:45:14.000-07:00
diff --git a/lib/v8debugapi.js b/lib/v8debugapi.js
@@ -59,6 +59,7 @@ module.exports.create = function(logger_, config_, fileStats_) {
   var config = null;
   var fileStats = null;
   var breakpoints = {};
+  // Entries map breakpoint id to { enabled: <bool>, listener: <function> }
   var listeners = {};
   var numBreakpoints = 0;
   // Before V8 4.5, having a debug listener active disables optimization. To
@@ -189,7 +190,7 @@ module.exports.create = function(logger_, config_, fileStats_) {
       var num = breakpoints[breakpoint.id].v8Breakpoint.number();
       var listener = onBreakpointHit.bind(
           null, breakpoint, function(err) {
-            delete listeners[num];
+            listeners[num].enabled = false;
             // This method is called from the debug event listener, which
             // swallows all exception. We defer the callback to make sure the
             // user errors aren't silenced.
@@ -198,7 +199,7 @@ module.exports.create = function(logger_, config_, fileStats_) {
             });
           });
 
-      listeners[num] = listener;
+      listeners[num] = { enabled: true, listener: listener };
     },
 
     /**
@@ -211,7 +212,7 @@ module.exports.create = function(logger_, config_, fileStats_) {
           null, breakpoint,
           createLogpointHandler(breakpoint, format, shouldStop, listener));
 
-      listeners[num] = listener;
+      listeners[num] = { enabled: true, listener: listener };
     },
 
     // The following are for testing:
@@ -341,12 +342,12 @@ module.exports.create = function(logger_, config_, fileStats_) {
       logsThisSecond++;
       console.log('LOGPOINT:', message);
       if (shouldStop()) {
-        delete listeners[num];
+        listeners[num].enabled = false;
       } else {
         if (logsThisSecond > config.log.maxLogsPerSecond) {
-          delete listeners[num];
+          listeners[num].enabled = false;
           setTimeout(function() {
-            listeners[num] = listener;
+            listeners[num].enabled = true;
           }, config.log.logDelaySeconds * 1000);
         }
       }
@@ -506,14 +507,20 @@ module.exports.create = function(logger_, config_, fileStats_) {
    * @param {Debug#BreakEvent} eventData
    */
   function handleDebugEvents(evt, execState, eventData) {
-    switch (evt) {
-      case v8.DebugEvent.Break:
-        eventData.breakPointsHit().forEach(function(hit) {
-          var num = hit.script_break_point().number();
-          logger.info('>>>V8 breakpoint hit<<< number: ' + num);
-          listeners[num](execState, eventData);
-        });
-        break;
+    try {
+      switch (evt) {
+        case v8.DebugEvent.Break:
+          eventData.breakPointsHit().forEach(function(hit) {
+            var num = hit.script_break_point().number();
+            if (listeners[num].enabled) {
+              logger.info('>>>V8 breakpoint hit<<< number: ' + num);
+              listeners[num].listener(execState, eventData);
+            }
+          });
+          break;
+      }
+    } catch (e) {
+      logger.warn('Internal V8 error on breakpoint event: ' + e);
     }
   }
 
diff --git a/test/test-v8debugapi.js b/test/test-v8debugapi.js
@@ -403,6 +403,29 @@ describe('v8debugapi', function() {
 
     });
 
+    it('should work with multiply hit breakpoints', function(done) {
+      var oldWarn = logger.warn;
+      var logCount = 0;
+      // If an exception is thrown we will log
+      logger.warn = function() { logCount++; };
+      // clone a clean breakpointInFoo
+      var bp = {id: breakpointInFoo.id, location: breakpointInFoo.location};
+      api.set(bp, function(err) {
+        assert.ifError(err);
+        api.wait(bp, function(err) {
+          assert.ifError(err);
+          setTimeout(function() {
+            logger.warn = oldWarn;
+            assert.equal(logCount, 0);
+            api.clear(bp);
+            done();
+          }, 100);
+        });
+        process.nextTick(function() {foo(1);});
+        setTimeout(function() {foo(2);}, 50);
+      });
+    });
+
     it('should be possible to wait on a logpoint without expressions',
         function(done) {
       var bp = {
