fix: periodically reset v8 session to prevent memory leak

src/agent/config.ts

@@ -357,6 +357,14 @@ 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 the
+   * breakpoints. This is to release the memory usage held by V8 engine for each
+   * breakpoint hit to prevent the memory leak. The default value is specified
+   * in defaultConfig.
+   */
+  resetV8DebuggerThreshold: number;
 }
 
 export interface StackdriverConfig extends GoogleAuthOptions {
@@ -406,4 +414,5 @@ export const defaultConfig: ResolvedDebugAgentConfig = {
 
   forceNewAgent_: false,
   testMode_: false,
+  resetV8DebuggerThreshold: 30,
 };

src/agent/controller.ts

@@ -134,7 +134,7 @@ export class Controller extends ServiceObject {
         } else if (response.statusCode === 404) {
           // The v2 API returns 404 (google.rpc.Code.NOT_FOUND) when the agent
           // registration expires. We should re-register.
-          callback(null, (response as {}) as t.Response);
+          callback(null, response as {} as t.Response);
           return;
         } else if (response.statusCode !== 200) {
           callback(
@@ -146,7 +146,7 @@ export class Controller extends ServiceObject {
         } else {
           body = body || {};
           that.nextWaitToken = body.nextWaitToken;
-          callback(null, (response as {}) as t.Response, body);
+          callback(null, response as {} as t.Response, body);
         }
       }
     );

src/agent/debuglet.ts

@@ -457,7 +457,7 @@ export class Debuglet extends EventEmitter {
     let sourceContext;
     try {
       sourceContext =
-        ((that.config.sourceContext as {}) as SourceContext) ||
+        (that.config.sourceContext as {} as SourceContext) ||
         (await Debuglet.getSourceContextFromFile());
     } catch (err5) {
       that.logger.warn('Unable to discover source context', err5);
@@ -741,9 +741,11 @@ export class Debuglet extends EventEmitter {
           );
           // TODO: Handle the case when `that.debuggee` is null.
           // TODO: Handle the case when `result` is undefined.
-          (that.debuggee as Debuggee).id = (result as {
-            debuggee: Debuggee;
-          }).debuggee.id;
+          (that.debuggee as Debuggee).id = (
+            result as {
+              debuggee: Debuggee;
+            }
+          ).debuggee.id;
           // TODO: Handle the case when `result` is undefined.
           that.emit('registered', (result as {debuggee: Debuggee}).debuggee.id);
           that.debuggeeRegistered.resolve();
@@ -926,9 +928,7 @@ export class Debuglet extends EventEmitter {
       //              field.  It is possible that breakpoint.id is always
       //              undefined!
       // TODO: Make sure the use of `that` here is correct.
-      delete that.completedBreakpointMap[
-        ((breakpoint as {}) as {id: number}).id
-      ];
+      delete that.completedBreakpointMap[(breakpoint as {} as {id: number}).id];
     });
 
     // Remove active breakpoints that the server no longer care about.
@@ -946,9 +946,9 @@ export class Debuglet extends EventEmitter {
    * @return {Object.<string, Breakpoint>} A map of breakpoint IDs to breakpoints.
    * @private
    */
-  convertBreakpointListToMap_(
-    breakpointList: stackdriver.Breakpoint[]
-  ): {[key: string]: stackdriver.Breakpoint} {
+  convertBreakpointListToMap_(breakpointList: stackdriver.Breakpoint[]): {
+    [key: string]: stackdriver.Breakpoint;
+  } {
     const map: {[id: string]: stackdriver.Breakpoint} = {};
     breakpointList.forEach(breakpoint => {
       // TODO: Address the case when `breakpoint.id` is `undefined`.
@@ -1098,13 +1098,15 @@ export class Debuglet extends EventEmitter {
     const that = this;
 
     // TODO: Address the case when `that.debuggee` is `null`.
-    that.controller.updateBreakpoint(that.debuggee as Debuggee, breakpoint, (
-      err /*, body*/
-    ) => {
-      if (err) {
-        that.logger.error('Unable to complete breakpoint on server', err);
+    that.controller.updateBreakpoint(
+      that.debuggee as Debuggee,
+      breakpoint,
+      (err /*, body*/) => {
+        if (err) {
+          that.logger.error('Unable to complete breakpoint on server', err);
+        }
       }
-    });
+    );
   }
 
   /**

src/agent/io/sourcemapper.ts

@@ -74,9 +74,9 @@ async function processSourcemap(
     // TODO: Resolve the cast of `contents as any` (This is needed because the
     //       type is expected to be of `RawSourceMap` but the existing
     //       working code uses a string.)
-    consumer = (new sourceMap.SourceMapConsumer(
-      (contents as {}) as sourceMap.RawSourceMap
-    ) as {}) as sourceMap.RawSourceMap;
+    consumer = new sourceMap.SourceMapConsumer(
+      contents as {} as sourceMap.RawSourceMap
+    ) as {} as sourceMap.RawSourceMap;
   } catch (e) {
     throw new Error(
       'An error occurred while reading the ' +
@@ -246,7 +246,8 @@ export class SourceMapper {
     };
 
     // TODO: Determine how to remove the explicit cast here.
-    const consumer: sourceMap.SourceMapConsumer = (entry.mapConsumer as {}) as sourceMap.SourceMapConsumer;
+    const consumer: sourceMap.SourceMapConsumer =
+      entry.mapConsumer as {} as sourceMap.SourceMapConsumer;
     const allPos = consumer.allGeneratedPositionsFor(sourcePos);
     /*
      * Based on testing, it appears that the following code is needed to
@@ -270,7 +271,7 @@ export class SourceMapper {
       // TODO: The `sourceMap.Position` type definition has a `column`
       //       attribute and not a `col` attribute.  Determine if the type
       //       definition or this code is correct.
-      column: ((mappedPos as {}) as {col: number}).col, // SourceMapConsumer uses
+      column: (mappedPos as {} as {col: number}).col, // SourceMapConsumer uses
       // zero-based column
       // numbers which is the
       // same as the expected

src/agent/state/inspector-state.ts

@@ -151,7 +151,7 @@ class StateResolver {
     };
 
     // TODO: Determine why _extend is used here
-    this.resolvedVariableTable = ((util as {}) as {_extend: Function})._extend(
+    this.resolvedVariableTable = (util as {} as {_extend: Function})._extend(
       [],
       this.messageTable
     );

src/agent/state/legacy-state.ts

@@ -138,7 +138,7 @@ class StateResolver {
     };
 
     // TODO: Determine why _extend is used here
-    this.resolvedVariableTable = ((util as {}) as {_extend: Function})._extend(
+    this.resolvedVariableTable = (util as {} as {_extend: Function})._extend(
       [],
       this.messageTable
     );
@@ -148,7 +148,7 @@ class StateResolver {
 
     // This constructor is only used in situations where the legacy vm
     // interface is used that has the `runInDebugContext` method.
-    this.scopeType = ((vm as {}) as LegacyVm).runInDebugContext('ScopeType');
+    this.scopeType = (vm as {} as LegacyVm).runInDebugContext('ScopeType');
   }
 
   /**

src/agent/util/debug-assert.ts

@@ -62,5 +62,5 @@ const fakeAssert: FakeAssert = {
 
 export function debugAssert(enableAssertions: boolean): FakeAssert {
   // The typecast is needed since the @types/node doesn't cover Node 10 yet
-  return enableAssertions ? ((realAssert as {}) as FakeAssert) : fakeAssert;
+  return enableAssertions ? (realAssert as {} as FakeAssert) : fakeAssert;
 }

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 {
@@ -559,9 +588,11 @@ export class InspectorDebugApi implements debugapi.DebugApi {
           // TODO: Address the case where `breakpoint.id` is `null`.
           breakpoint.expressions[i] =
             // TODO: Address the case where `compile` is `null`.
-            (this.breakpoints[breakpoint.id].compile as (
-              text: string
-            ) => string)(breakpoint.expressions[i]);
+            (
+              this.breakpoints[breakpoint.id].compile as (
+                text: string
+              ) => string
+            )(breakpoint.expressions[i]);
         } catch (e) {
           this.logger.info(
             'Unable to compile watch expression >> ' +
@@ -591,7 +622,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 +637,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
         breakpoint,
         this.config,
         this.scriptMapper,
-        this.v8Inspector
+        this.v8.inspector
       );
       if (
         breakpoint.location &&
@@ -617,7 +648,8 @@ export class InspectorDebugApi implements debugapi.DebugApi {
       breakpoint.stackFrames = captured.stackFrames;
       // TODO: This suggests the Status type and Variable type are the same.
       //       Determine if that is the case.
-      breakpoint.variableTable = captured.variableTable as stackdriver.Variable[];
+      breakpoint.variableTable =
+        captured.variableTable as stackdriver.Variable[];
       breakpoint.evaluatedExpressions = expressionErrors.concat(
         captured.evaluatedExpressions
       );
@@ -639,5 +671,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);
+      }
+    }
   }
 }