Skip to content

Guard the elements array usage in ElementTextCache to address the memory leak#285

Merged
atootoonchianmeta merged 1 commit intofacebook:mainfrom
sudojorian:fix-elementtextcache-memory-leak
Feb 11, 2026
Merged

Guard the elements array usage in ElementTextCache to address the memory leak#285
atootoonchianmeta merged 1 commit intofacebook:mainfrom
sudojorian:fix-elementtextcache-memory-leak

Conversation

@sudojorian
Copy link
Copy Markdown
Contributor

@sudojorian sudojorian commented Feb 11, 2026

Context

Using memlab, we identified that a significant memory leak is coming from ElementTextCache as it's holding the detached DOM elements. Below is the memory trace

--Retained size of leaked objects: 1MB--
[Window ] (native) @598547 [324.3KB]
  --global_object (shortcut)--->  [Window (global*) / https://adsmanager.42069.od.facebook.com/] (object) @954191 [51.4KB]
  --ALEAMainWindowContext (property)--->  [Object] (object) @957233 [2.4KB]
  --hyperion (property)--->  [Object] (object) @3929347 [312 bytes]
  --AutoLogging (property)--->  [Object] (object) @989871 [292 bytes]
  --init (property)--->  [init] (closure) @3930175 [80 bytes]
  --context (internal)--->  [<function scope>] (object) @1453993 [20.7KB]
  --ElementTextCache (variable)--->  [WeakMap] (object) @3929639 [1.1KB]
  --table (internal)--->  [<array>] (array) @3929821 [1KB]
  --21 / part of key (HTMLDivElement @581803) -> value (Object @5964475) pair in WeakMap (table @3929821) (internal)--->  [Object] (object) @5964475 [236 bytes]
  --result (property)--->  [Object] (object) @6211775 [216 bytes]
  --elementText (property)--->  [Object] (object) @6211771 [196 bytes]
  --elements (property)--->  [Array] (object) @6211763 [28 bytes]
  --0 (element)--->  [Detached <div data-testids=" | GeoALSurface | GeoALSurface [from GeoALSurface] | GeoBaseText | GeoB... (native) @581815 [1KB]
  --11 (element)--->  [Detached <div data-testids=" | GeoALSurface | GeoALSurface [from GeoALSurface] | GeoFlexbox | GeoBa... (native) @581813 [664 bytes]
  --9 (element)--->  [Detached <div data-testids=" | GeoALSurface | GeoALSurface [from GeoALSurface] | GeoBaseText | GeoB... (native) @581811 [664 bytes]
  --9 (element)--->  [Detached <span data-testids=" | GeoALSurface | GeoALSurface [from GeoALSurface] | GeoBaseText | Geo... (native) @581805 [664 bytes]
  --9 (element)--->  [Detached <div role="button" data-testid="ads_report_builder_geo_custom_metric_dialog_button_b71d" d... (native) @581803 [872 bytes]
  --11 (element)--->  [Detached <div role="none" data-testids=" | SurfaceWithEvent | SurfaceImpl | Surface | Surface [from... (native) @7119 [708 bytes]

Problem

The ElementTextCache is a WeakMap that caches element text extraction results:

// Line 785-791
let ElementTextCache = null;
function init$6(options) {
    if (options.enableElementTextCache) {
        ElementTextCache = new WeakMap();  // Good - WeakMap for keys
    }
}

However, the cached value contains strong references that prevent garbage collection:

// Lines 1023-1026
ElementTextCache?.set(element, {
    surface,              // Strong reference to ALSurfaceData object
    result: finalResult,  // Contains elementText.elements array
});

The finalResult.elementText.elements array accumulates all DOM elements that contributed text (see lines 1010):

elements: [...prev.elements, ...current.elements]  // Grows with each child element

The elements array accumulated all DOM elements that contributed text to the cached result. When an element was removed from the DOM.

Fix

This diff fixes a memory leak by guarding the elements array which contains the DOM element references from cached text objects. When excludeElementsFromCache option is enabled, it creates leaner cache entries containing only text and source properties instead of also storing element references.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 11, 2026
@sudojorian sudojorian marked this pull request as ready for review February 11, 2026 02:50
@atootoonchianmeta atootoonchianmeta merged commit 4bb0aed into facebook:main Feb 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants