fix: periodically reset v8 session to prevent memory leak

src/agent/config.ts

@@ -357,6 +357,13 @@ export interface ResolvedDebugAgentConfig extends GoogleAuthOptions {
    * used to set a default api url
    */
   apiUrl?: string;
+
+  /**
+   * Number of times of the V8 breakpoint hits events before resetting (disable and
+   * re-enable) the breakpoints. This is to release the memory usage held by V8
+   * engine for each breakpoint hit to prevent the memory leak.
+   */
+  resetV8DebuggerThreshold: number;
 }
 
 export interface StackdriverConfig extends GoogleAuthOptions {
@@ -406,4 +413,5 @@ export const defaultConfig: ResolvedDebugAgentConfig = {
 
   forceNewAgent_: false,
   testMode_: false,
+  resetV8DebuggerThreshold: 30,
 };

src/agent/debuglet.ts

@@ -312,6 +312,7 @@ export class Debuglet extends EventEmitter {
         minorVersion_:
           process.env.GAE_DEPLOYMENT_ID || process.env.GAE_MINOR_VERSION,
       },
+      resetV8DebuggerThreshold: process.env.RESET_V8_DEBUGGER_THRESHOLD,
     };
 
     if (process.env.FUNCTION_NAME) {

src/agent/v8/inspector-debugapi.ts

@@ -41,6 +41,21 @@ interface InspectorOptions {
   useWellFormattedUrl: boolean;
 }
 
+/** Data related to the v8 inspector. */
+interface V8Data {
+  session: inspector.Session;
+  // Options for behavior when interfacing with the Inspector API.
+  inspectorOptions: InspectorOptions;
+  inspector: V8Inspector;
+  // Store the v8 setBreakpoint parameters for each v8 breakpoint so that later
+  // the recorded parameters can be used to reset the breakpoints.
+  setBreakpointsParams: {
+    [
+      v8BreakpointId: string
+    ]: inspector.Debugger.SetBreakpointByUrlParameterType;
+  };
+}
+
 /**
  * In older versions of Node, the script source as seen by the Inspector
  * backend is wrapped in `require('module').wrapper`, and in new versions
@@ -68,7 +83,6 @@ export class InspectorDebugApi implements debugapi.DebugApi {
   fileStats: ScanStats;
   breakpoints: {[id: string]: BreakpointData} = {};
   sourcemapper: SourceMapper;
-  session: inspector.Session;
   // TODO: listeners, scrpitmapper, location mapper and breakpointmapper can use
   // Map in the future after resolving Map.prototype.get(key) returns V or
   // undefined.
@@ -81,9 +95,9 @@ export class InspectorDebugApi implements debugapi.DebugApi {
   // stackdriver breakpoint id.
   breakpointMapper: {[id: string]: stackdriver.BreakpointId[]} = {};
   numBreakpoints = 0;
-  // Options for behavior when interfacing with the Inspector API.
-  private inspectorOptions: InspectorOptions;
-  v8Inspector: V8Inspector;
+  numBreakpointHitsBeforeReset = 0;
+  v8: V8Data;
+
   constructor(
     logger: consoleLogLevel.Logger,
     config: ResolvedDebugAgentConfig,
@@ -94,25 +108,36 @@ export class InspectorDebugApi implements debugapi.DebugApi {
     this.config = config;
     this.fileStats = jsFiles;
     this.sourcemapper = sourcemapper;
-    this.session = new inspector.Session();
-    this.session.connect();
-    this.session.on('Debugger.scriptParsed', script => {
+    this.scriptMapper = {};
+    this.v8 = this.createV8Data();
+  }
+
+  /** Creates a new V8 Debugging session and the related data. */
+  private createV8Data(): V8Data {
+    const session = new inspector.Session();
+    session.connect();
+    session.on('Debugger.scriptParsed', script => {
       this.scriptMapper[script.params.scriptId] = script.params;
     });
-    this.session.post('Debugger.enable');
-    this.session.post('Debugger.setBreakpointsActive', {active: true});
-    this.session.on('Debugger.paused', message => {
+    session.post('Debugger.enable');
+    session.post('Debugger.setBreakpointsActive', {active: true});
+    session.on('Debugger.paused', message => {
       try {
         this.handleDebugPausedEvent(message.params);
       } catch (error) {
         this.logger.error(error);
       }
     });
-    this.inspectorOptions = {
-      // Well-Formatted URL is required in Node 10.11.1+.
-      useWellFormattedUrl: utils.satisfies(process.version, '>10.11.0'),
+
+    return {
+      session,
+      inspectorOptions: {
+        // Well-Formatted URL is required in Node 10.11.1+.
+        useWellFormattedUrl: utils.satisfies(process.version, '>10.11.0'),
+      },
+      inspector: new V8Inspector(session),
+      setBreakpointsParams: {},
     };
-    this.v8Inspector = new V8Inspector(this.session);
   }
 
   set(
@@ -219,7 +244,8 @@ export class InspectorDebugApi implements debugapi.DebugApi {
     if (!this.breakpointMapper[breakpointData.id]) {
       // When breakpointmapper does not countain current v8/inspector breakpoint
       // id, we should remove this breakpoint from v8.
-      result = this.v8Inspector.removeBreakpoint(breakpointData.id);
+      result = this.v8.inspector.removeBreakpoint(breakpointData.id);
+      delete this.v8.setBreakpointsParams[breakpointData.id];
     }
     delete this.breakpoints[breakpoint.id];
     delete this.listeners[breakpoint.id];
@@ -297,7 +323,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
   }
 
   disconnect(): void {
-    this.session.disconnect();
+    this.v8.session.disconnect();
   }
 
   numBreakpoints_(): number {
@@ -487,7 +513,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
     let v8BreakpointId; // v8/inspector breakpoint id
     if (!this.locationMapper[locationStr]) {
       // The first time when a breakpoint was set to this location.
-      const rawUrl = this.inspectorOptions.useWellFormattedUrl
+      const rawUrl = this.v8.inspectorOptions.useWellFormattedUrl
         ? `file://${matchingScript}`
         : matchingScript;
       // on windows on Node 11+, the url must start with file:///
@@ -496,17 +522,20 @@ export class InspectorDebugApi implements debugapi.DebugApi {
         process.platform === 'win32' && utils.satisfies(process.version, '>=11')
           ? rawUrl.replace(/^file:\/\//, 'file:///').replace(/\\/g, '/')
           : rawUrl;
-      const res = this.v8Inspector.setBreakpointByUrl({
+      const params = {
         lineNumber: line - 1,
         url,
         columnNumber: column - 1,
         condition: breakpoint.condition || undefined,
-      });
+      };
+      const res = this.v8.inspector.setBreakpointByUrl(params);
       if (res.error || !res.response) {
         // Error case.
         return null;
       }
       v8BreakpointId = res.response.breakpointId;
+      this.v8.setBreakpointsParams[v8BreakpointId] = params;
+
       this.locationMapper[locationStr] = [];
       this.breakpointMapper[v8BreakpointId] = [];
     } else {
@@ -591,7 +620,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
         const evaluatedExpressions = breakpoint.expressions.map(exp => {
           // returnByValue is set to true here so that the JSON string of the
           // value will be returned to log.
-          const result = state.evaluate(exp, frame, that.v8Inspector, true);
+          const result = state.evaluate(exp, frame, that.v8.inspector, true);
           if (result.error) {
             return result.error;
           } else {
@@ -606,7 +635,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
         breakpoint,
         this.config,
         this.scriptMapper,
-        this.v8Inspector
+        this.v8.inspector
       );
       if (
         breakpoint.location &&
@@ -639,5 +668,37 @@ export class InspectorDebugApi implements debugapi.DebugApi {
     } catch (e) {
       this.logger.warn('Internal V8 error on breakpoint event: ' + e);
     }
+
+    this.tryResetV8Debugger();
+  }
+
+  /**
+   * Periodically resets breakpoints to prevent memory leaks in V8 (for holding
+   * contexts of previous breakpoint hits).
+   */
+  private tryResetV8Debugger() {
+    this.numBreakpointHitsBeforeReset += 1;
+    if (
+      this.numBreakpointHitsBeforeReset < this.config.resetV8DebuggerThreshold
+    ) {
+      return;
+    }
+    this.numBreakpointHitsBeforeReset = 0;
+
+    const storedParams = this.v8.setBreakpointsParams;
+
+    // Re-connect the session to clean the memory usage.
+    this.disconnect();
+    this.scriptMapper = {};
+    this.v8 = this.createV8Data();
+    this.v8.setBreakpointsParams = storedParams;
+
+    // Setting the v8 breakpoints again according to the stored parameters.
+    for (const params of Object.values(storedParams)) {
+      const res = this.v8.inspector.setBreakpointByUrl(params);
+      if (res.error || !res.response) {
+        this.logger.error('Error upon re-setting breakpoint: ' + res);
+      }
+    }
   }
 }

test/test-debuglet.ts

@@ -624,6 +624,15 @@ describe('Debuglet', () => {
         });
         debuglet.start();
       });
+
+      it('should respect RESET_V8_DEBUGGER_THRESHOLD env. var. ', () => {
+        process.env.RESET_V8_DEBUGGER_THRESHOLD = '123';
+        const debug = new Debug({}, packageInfo);
+        const debuglet = new Debuglet(debug, defaultConfig);
+        assert.ok(debuglet.config);
+        assert.ok(debuglet.config.resetV8DebuggerThreshold);
+        assert.strictEqual(debuglet.config.resetV8DebuggerThreshold, '123');
+      });
     });
 
     it('should retry on failed registration', function (done) {

test/test-v8debugapi.ts

@@ -35,6 +35,7 @@ import * as extend from 'extend';
 import * as debugapi from '../src/agent/v8/debugapi';
 import {defaultConfig} from '../src/agent/config';
 import {StatusMessage} from '../src/client/stackdriver/status-message';
+import {InspectorDebugApi} from '../src/agent/v8/inspector-debugapi';
 import * as scanner from '../src/agent/io/scanner';
 import * as SourceMapper from '../src/agent/io/sourcemapper';
 import * as path from 'path';
@@ -722,6 +723,76 @@ describe('v8debugapi', () => {
     });
   });
 
+  describe('InspectorDebugApi', () => {
+    let oldLPS: number;
+    let oldDS: number;
+
+    before(() => {
+      oldLPS = config.log.maxLogsPerSecond;
+      oldDS = config.log.logDelaySeconds;
+      config.log.maxLogsPerSecond = config.resetV8DebuggerThreshold * 3;
+      config.log.logDelaySeconds = 1;
+    });
+
+    after(() => {
+      config.log.maxLogsPerSecond = oldLPS;
+      config.log.logDelaySeconds = oldDS;
+      assert(stateIsClean(api));
+    });
+
+    it('should perform v8 breakpoints reset when meeting threshold', done => {
+      // The test is only eligible for the InspectorDebugApi test target.
+      if (!(api instanceof InspectorDebugApi)) {
+        done();
+        return;
+      }
+
+      const bp: stackdriver.Breakpoint = ({
+        id: breakpointInFoo.id,
+        location: breakpointInFoo.location,
+        action: 'LOG',
+        logMessageFormat: 'cat',
+      } as {}) as stackdriver.Breakpoint;
+      api.set(bp, err1 => {
+        let logpointEvaluatedTimes = 0;
+
+        assert.ifError(err1);
+        api.log(
+          bp,
+          () => {
+            logpointEvaluatedTimes += 1;
+          },
+          () => false
+        );
+
+        const inspectorDebugApi = api as InspectorDebugApi;
+        const v8BeforeReset = inspectorDebugApi.v8;
+
+        // The loop should trigger the breakpoints reset.
+        for (let i = 0; i < config.resetV8DebuggerThreshold; i++) {
+          code.foo(1);
+        }
+
+        // Expect the current v8 data is no longer the previous one.
+        assert.notStrictEqual(inspectorDebugApi.v8, v8BeforeReset);
+
+        // Make sure the logpoint is still triggered correctly after the second reset.
+        for (let i = 0; i < config.resetV8DebuggerThreshold + 1; i++) {
+          code.foo(1);
+        }
+        assert.strictEqual(
+          logpointEvaluatedTimes,
+          config.resetV8DebuggerThreshold * 2 + 1
+        );
+
+        api.clear(bp, err2 => {
+          assert.ifError(err2);
+          done();
+        });
+      });
+    });
+  });
+
   describe('set and wait', () => {
     it('should be possible to wait on a breakpoint', done => {
       // clone a clean breakpointInFoo