Prevent external retainer trace edge filter from OOMing memlab path f…

…inder (#117)

Summary:
External leak filter's `retainerReferenceFilter` callback could return `true` for edges that shouldn't be used. For example, self-referencing edges cause to infinite loop when traversing from a node to the GC root. MemLab's default edge filter excludes these kind of edges, but MemLab's path finder didn't consider the case where external leak filter may bypass MemLab's internal edge filter.

This diff makes a patch to MemLab so that its path finder will consider the case where external edge filter includes edges that should be bypassed.

Differential Revision: D56803842

fbshipit-source-id: d25ab366c5a1562927d284299a0bcfb057d9330a

packages/core/src/lib/Types.ts

@@ -509,11 +509,17 @@ export interface ILeakFilter {
    * therefore, the reference and heap object will not be included in the
    * retainer trace detection and retainer size calculation.
    *
+   * Please also be aware that some edges like self-referencing edges,
+   * JS engine's internal edges, and hidden edges should not be considered
+   * as part of the retainer trace. These edges could make the retainer trace
+   * unncessarily complex and cause confusion. `isReferenceUsedByDefault` will
+   * be `false` for these types of edges.
+   *
    * * **Examples**:
    * ```javascript
    * // save as leak-filter.js
    * module.exports = {
-   *   retainerReferenceFilter(edge, _snapshot, _leakedNodeIds) {
+   *   retainerReferenceFilter(edge, _snapshot, _isReferenceUsedByDefault) {
    *     // exclude react fiber references
    *     if (edge.name_or_index.toString().startsWith('__reactFiber$')) {
    *       return false;
@@ -596,10 +602,16 @@ export type LeakFilterCallback = (
  * @returns the value indicating whether the given reference should be
  * filtered (i.e., included)
  *
+ * Please also be aware that some edges like self-referencing edges,
+ * JS engine's internal edges, and hidden edges should not be considered
+ * as part of the retainer trace. These edges could make the retainer trace
+ * unncessarily complex and cause confusion. `isReferenceUsedByDefault` will
+ * be `false` for these types of edges.
+ *
  * * **Examples**:
  * ```javascript
  * // exclude react fiber references
- * function retainerReferenceFilter(edge, _snapshot, _leakedNodeIds) {
+ * function retainerReferenceFilter(edge, _snapshot, _isReferenceUsedByDefault) {
  *   if (edge.name_or_index.toString().startsWith('__reactFiber$')) {
  *     return false;
  *   }

packages/core/src/lib/Utils.ts

@@ -686,7 +686,10 @@ function loadLeakFilter(filename: string): ILeakFilter {
     if (typeof filter === 'function') {
       return {leakFilter: filter};
     }
-    if (typeof filter?.leakFilter === 'function') {
+    if (
+      typeof filter?.leakFilter === 'function' ||
+      typeof filter?.retainerReferenceFilter === 'function'
+    ) {
       return filter;
     }
     throw haltOrThrow(`Invalid leak filter in ${filepath}`);

packages/core/src/paths/TraceFinder.ts

@@ -727,10 +727,16 @@ class TraceFinder {
     if (!node || !node.hasPathEdge) {
       return null;
     }
+    const visited = new Set<number>([node.id]);
     let path: LeakTracePathItem = {node};
     while (node && node.hasPathEdge) {
       const edge: IHeapEdge = node.pathEdge as IHeapEdge;
-      path = {node: edge.fromNode, edge, next: path};
+      const fromNode = edge.fromNode;
+      if (visited.has(fromNode.id)) {
+        return null;
+      }
+      visited.add(fromNode.id);
+      path = {node: fromNode, edge, next: path};
       node = edge.fromNode;
     }
     return path;

website/docs/api/interfaces/core_src.IBrowserInfo.md

@@ -16,7 +16,7 @@ through [RunMetaInfo](../modules/core_src.md#runmetainfo).
 browser version
 
  * **Source**:
-    * core/src/lib/Types.ts:1220
+    * core/src/lib/Types.ts:1232
 
 ___
 
@@ -25,7 +25,7 @@ ___
 all web console output
 
  * **Source**:
-    * core/src/lib/Types.ts:1228
+    * core/src/lib/Types.ts:1240
 
 ___
 
@@ -34,4 +34,4 @@ ___
 configuration for puppeteer
 
  * **Source**:
-    * core/src/lib/Types.ts:1224
+    * core/src/lib/Types.ts:1236

website/docs/api/interfaces/core_src.IHeapEdge.md

@@ -45,7 +45,7 @@ import {getFullHeapFromFile} from '@memlab/heap-analysis';
 index of this JS reference inside the `edge.snapshot.edges` pseudo array
 
  * **Source**:
-    * core/src/lib/Types.ts:1648
+    * core/src/lib/Types.ts:1660
 
 ___
 
@@ -55,7 +55,7 @@ returns an [IHeapNode](core_src.IHeapNode.md) instance representing the hosting
 JS heap object where this reference starts
 
  * **Source**:
-    * core/src/lib/Types.ts:1669
+    * core/src/lib/Types.ts:1681
 
 ___
 
@@ -67,7 +67,7 @@ otherwise this is a reference with a string name (`edge.name_or_index`
 will return a string)
 
  * **Source**:
-    * core/src/lib/Types.ts:1655
+    * core/src/lib/Types.ts:1667
 
 ___
 
@@ -77,7 +77,7 @@ name of the JS reference. If this is a reference to an array element
 or internal table element, it is an numeric index
 
  * **Source**:
-    * core/src/lib/Types.ts:1604
+    * core/src/lib/Types.ts:1616
 
 ___
 
@@ -86,7 +86,7 @@ ___
 get the [IHeapSnapshot](core_src.IHeapSnapshot.md) containing this JS reference
 
  * **Source**:
-    * core/src/lib/Types.ts:1644
+    * core/src/lib/Types.ts:1656
 
 ___
 
@@ -96,7 +96,7 @@ returns an [IHeapNode](core_src.IHeapNode.md) instance representing the JS heap
 pointed to by this reference
 
  * **Source**:
-    * core/src/lib/Types.ts:1664
+    * core/src/lib/Types.ts:1676
 
 ___
 
@@ -105,7 +105,7 @@ ___
 the index of the JS heap object pointed to by this reference
 
  * **Source**:
-    * core/src/lib/Types.ts:1659
+    * core/src/lib/Types.ts:1671
 
 ___
 
@@ -115,7 +115,7 @@ type of the JS reference, all types:
 `context`, `element`, `property`, `internal`, `hidden`, `shortcut`, `weak`
 
  * **Source**:
-    * core/src/lib/Types.ts:1609
+    * core/src/lib/Types.ts:1621
 
 ## Methods
 
@@ -136,4 +136,4 @@ captured by the hosting object.
     * `...args`: `any`[]
  * **Returns**: `string`
  * **Source**:
-    * core/src/lib/Types.ts:1682
+    * core/src/lib/Types.ts:1694

website/docs/api/interfaces/core_src.IHeapEdges.md

@@ -41,7 +41,7 @@ The total number of edges in heap graph (or JS references in heap
 snapshot).
 
  * **Source**:
-    * core/src/lib/Types.ts:1719
+    * core/src/lib/Types.ts:1731
 
 ## Methods
 
@@ -54,7 +54,7 @@ to each element in ascending order of element index.
     * `callback`: (`edge`: [`IHeapEdge`](core_src.IHeapEdge.md), `index`: `number`) => `boolean` \| `void` | the callback does not need to return any value, if the callback returns `false` when iterating on element at index `i`, then all elements after `i` won't be iterated.
  * **Returns**: `void`
  * **Source**:
-    * core/src/lib/Types.ts:1735
+    * core/src/lib/Types.ts:1747
 
 ___
 
@@ -68,4 +68,4 @@ get an [IHeapEdge](core_src.IHeapEdge.md) element at the specified index
 at the specified index, otherwise it returns `null`.
 
  * **Source**:
-    * core/src/lib/Types.ts:1727
+    * core/src/lib/Types.ts:1739

website/docs/api/interfaces/core_src.IHeapLocation.md

@@ -43,7 +43,7 @@ import {getFullHeapFromFile} from '@memlab/heap-analysis';
 get the column number
 
  * **Source**:
-    * core/src/lib/Types.ts:1582
+    * core/src/lib/Types.ts:1594
 
 ___
 
@@ -52,7 +52,7 @@ ___
 get the line number
 
  * **Source**:
-    * core/src/lib/Types.ts:1578
+    * core/src/lib/Types.ts:1590
 
 ___
 
@@ -61,7 +61,7 @@ ___
 get the heap object this location this location represents
 
  * **Source**:
-    * core/src/lib/Types.ts:1570
+    * core/src/lib/Types.ts:1582
 
 ___
 
@@ -70,7 +70,7 @@ ___
 get the script ID of the source file
 
  * **Source**:
-    * core/src/lib/Types.ts:1574
+    * core/src/lib/Types.ts:1586
 
 ___
 
@@ -79,7 +79,7 @@ ___
 get the [IHeapSnapshot](core_src.IHeapSnapshot.md) containing this location instance
 
  * **Source**:
-    * core/src/lib/Types.ts:1566
+    * core/src/lib/Types.ts:1578
 
 ## Methods
 
@@ -100,4 +100,4 @@ captured by the hosting object.
     * `...args`: `any`[]
  * **Returns**: `string`
  * **Source**:
-    * core/src/lib/Types.ts:1595
+    * core/src/lib/Types.ts:1607