__ngContext__ magnifies native Chrome memory leaks

[blidblid]

🐞 bug report

Affected Package

@angular/core@11.2.3

Is this a regression?

No.

Description

Small memory leaks become large leaks when Angular monkey patches __ngContext__ onto dom.

For example, take this issue. After typing into textareas, the issue leads to detached HTMLTextAreaElements because of the native undo stack.

It is a small leak that has nothing to do with Angular. It is only a few KBs:

But, with Angular components in the mix, those KBs become MBs:

It looks like __ngContext__, retains almost everything, even though the textarea is a vanilla element:

And if we remove this call from @angular/core:

🔬 Minimal Reproduction

https://github.com/blidblid/ng-context-memory-leak

1. ng s --prod
2. Type in the textarea to create the Chrome leak
3. Press Destroy and create
4. Repeat with @angular/material components included

The issue is also present on material.angular.io. Typing in a textarea and navigating the sidenav will cause large leaks.

🌍 Your Environment

Angular Version:

Angular CLI: 11.2.2
Node: 14.15.0
OS: win32 x64

Angular: 11.2.3
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1100.7
@angular-devkit/build-angular   0.1100.7
@angular-devkit/core            11.0.7
@angular-devkit/schematics      11.2.2
@angular/cdk                    11.2.2
@angular/cli                    11.2.2
@angular/material               11.2.2
@schematics/angular             11.2.2
@schematics/update              0.1102.2
rxjs                            6.6.6
typescript                      4.0.7

Anything else relevant?

Chrome 88.0.4324.190

[blidblid]

My team is working around this by patching attachPatchData to:

export function attachPatchData(target: any, data: LView|LContext) {
  if (ngDevMode || !(target instanceof EventTarget)) {
    target[MONKEY_PATCH_KEY_NAME] = data;
  }
}

With the change, our stress tests leak 50mb instead of 325mb. The question is, are production Angular apps reliant on DOM-nodes getting patched with __ngContext__? From looking at E2E tests, it does not seem so.

[jelbourn]

@mhevery is __ngContext__ a devmode only thing, or is this necessary for the framework to function normally?

[NordMagnus]

Any updates on this?

[mhevery]

@mhevery is __ngContext__ a devmode only thing, or is this necessary for the framework to function normally?

It is necessary for some APIs, so it is not dev only.

My understanding is that this is an issue with Chrome and there is not much we can do about this.

[AndrewKushnir]

Closing this ticket based on Misko's reply above. Detached DOM elements are retained in memory due to Chrome bug and these elements refer to some internal Angular data structures that become non-garbage-collected because of that. Once the issue is resolved in Chrome, the memory leak should be gone.

[kallebjork]

This is quite an issue for any larger projects migrating to Ivy, since it is not only the chrome issue. Any existing leaks (detached DOM nodes) will grow substantially and if you are deploying in a shared environment with limited RAM, this has quite an impact.

We should fix any leaks we have but the chrome issue we cannot fix and it looks to me like the monkey patch also makes the process of finding the leaks much more difficult, since everything is retain by ngContext?

If we are right in that the monkey patch increases any existing leaks and that it prevents us from finding the real retainers, closing this issue means any larger projects should stick with view engine until they have fixed all their leaks (the chrome issue not even possible to fix). We have already migrated and are left with patching angular core, both to reduce the leaks in size and to be able to get the real retainers. Are we missing something here?

I think you currently have this issue on angular.io. Not sure if it is only because of the chrome issue or if you have other leaks as well.

[NordMagnus]

Besides that I agree with @kallebjork I'm also curious about what APIs need this in production and why.

[blidblid]

@mhevery Does this also apply to patching native elements specifically?

From our perspective, this issue would put 100.000 healthcare workers on ViewEngine indefinitely. That's a huge blow, because Ivy is amazing and because we'll be stranded with an old Angular version. The background here is that the hospital environments usually have 100-200 MB of RAM. And if the framework magnifies leaks by a factor thousand, those environments will go down.

I get that the reproduction above uses a Chrome issue, but the role of __ngContext__ as a massive retainer applies to any leak. Angular has 13 open memory leaks. We have some, rxjs has some, Chrome has some. Any large app will leak a bit, and that's fine as long as those leaks are small and self-contained. __ngContext__ removes that containment.

I'm happy to rephrase the issue to "ngContext magnifies memory leaks" if you think that is a better angle here.

[JiaLiPassion]

This may related to the issue here #36011, it seems to be an issue from chrome https://bugs.chromium.org/p/v8/issues/detail?id=10297#c5, and maybe we can provide a API to clear the ngContext?

[jelbourn]

We discussed this some more and think we might be able to use requestIdleCallback to directly clear __ngContext__ (and mitigate DOM memory leaks outside our control) without incurring runtime performance penalty. We'd definitely have to try this out with a number of different apps, though, to see the real-world impact.

[crisbeto]

@denspi see the comment above, we ended up reverting the fix due to concerns around backwards compatibility.

[denspi]

@denspi see the comment above, we ended up reverting the fix due to concerns around backwards compatibility.

Thanks, just wanted to make sure I havent missed something.

[KimAlexander]

@crisbeto it is a way that you merge #41358 in angular 9 version ?

[crisbeto]

@KimAlexander we ended up reverting #41358 due to some issues. See the discussion above.

[codebreach]

Any ETA on a fix for this?

[denspi]

@codebreach I just created a directive which in ngOnDestroy gets ngContext, finds AnimationRendererFactory.engine._transitionEngine and clears statesByElement and playersByElement arrays. Works fine by now.

[blidblid]

For anybody looking for an ugly workaround, I've yet to see any issues with this patch of attachPatchData:

import { process as runNgcc } from '@angular/compiler-cli/ngcc';
import { existsSync, readFileSync, writeFileSync } from 'fs';
import { join } from 'path';

const ANGULAR_CORE_PATH = '@angular/core';
const ANGULAR_CORE_NGCC_PATH = `${ANGULAR_CORE_PATH}/__ivy_ngcc__/fesm2015/core.js`;

const REPLACE_AT = `
function attachPatchData(target, data) {
    ngDevMode && assertDefined(target, 'Target expected');
    target[MONKEY_PATCH_KEY_NAME] = data;
}
`.trim();

const REPLACE_WITH = `
function attachPatchData(target, data) {
    ngDevMode && assertDefined(target, 'Target expected');
    if (ngDevMode || !(target instanceof EventTarget)) {
      target[MONKEY_PATCH_KEY_NAME] = data;
    }
}
`.trim();

// https://github.com/angular/angular/issues/41047
export function patchAngularToRemoveNgContextFromDom(nodeModulesPath: string): void {
  const patchFilePath = join(nodeModulesPath, ANGULAR_CORE_NGCC_PATH);

  if (!existsSync(patchFilePath)) {
    runNgcc({
      basePath: nodeModulesPath,
      targetEntryPointPath: join(nodeModulesPath, ANGULAR_CORE_PATH),
      createNewEntryPointFormats: true,
      propertiesToConsider: ['fesm2015'],
    });

    if (!existsSync(patchFilePath)) {
      throw new Error(`Tried to patch @angular/core but could not find ${patchFilePath}.`);
    }
  }

  const fileContent = readFileSync(patchFilePath, 'utf8');
  const replacedFileContent = fileContent.replace(REPLACE_AT, REPLACE_WITH);

  if (!replacedFileContent.includes(REPLACE_WITH)) {
    throw new Error(`Could not patch memory leak in @angular/core.`);
  }

  if (fileContent.length === replacedFileContent.length) {
    return;
  }

  writeFileSync(patchFilePath, replacedFileContent);
}

[merrillpaul]

For anybody looking for an ugly workaround, I've yet to see any issues with this patch of attachPatchData:

import { process as runNgcc } from '@angular/compiler-cli/ngcc';
import { existsSync, readFileSync, writeFileSync } from 'fs';
import { join } from 'path';

const ANGULAR_CORE_PATH = '@angular/core';
const ANGULAR_CORE_NGCC_PATH = `${ANGULAR_CORE_PATH}/__ivy_ngcc__/fesm2015/core.js`;

const REPLACE_AT = `
function attachPatchData(target, data) {
    ngDevMode && assertDefined(target, 'Target expected');
    target[MONKEY_PATCH_KEY_NAME] = data;
}
`.trim();

const REPLACE_WITH = `
function attachPatchData(target, data) {
    ngDevMode && assertDefined(target, 'Target expected');
    if (ngDevMode || !(target instanceof EventTarget)) {
      target[MONKEY_PATCH_KEY_NAME] = data;
    }
}
`.trim();

// https://github.com/angular/angular/issues/41047
export function patchAngularToRemoveNgContextFromDom(nodeModulesPath: string): void {
  const patchFilePath = join(nodeModulesPath, ANGULAR_CORE_NGCC_PATH);

  if (!existsSync(patchFilePath)) {
    runNgcc({
      basePath: nodeModulesPath,
      targetEntryPointPath: join(nodeModulesPath, ANGULAR_CORE_PATH),
      createNewEntryPointFormats: true,
      propertiesToConsider: ['fesm2015'],
    });

    if (!existsSync(patchFilePath)) {
      throw new Error(`Tried to patch @angular/core but could not find ${patchFilePath}.`);
    }
  }

  const fileContent = readFileSync(patchFilePath, 'utf8');
  const replacedFileContent = fileContent.replace(REPLACE_AT, REPLACE_WITH);

  if (!replacedFileContent.includes(REPLACE_WITH)) {
    throw new Error(`Could not patch memory leak in @angular/core.`);
  }

  if (fileContent.length === replacedFileContent.length) {
    return;
  }

  writeFileSync(patchFilePath, replacedFileContent);
}

You are a life saver! With a similar fix our app is now smooth and performant on even the slower iOs Air2s ( ours is a hybrid ionic Ng app ). Basically we have 100s of cards that can be paginated back and forth and the dom nodes and memory were climbing up. We added this "hack" and the UI is now much better than ever.
Our diffs for the patch were

6296a6297
>     // testing FESM
6298c6299,6301
<     target[MONKEY_PATCH_KEY_NAME] = data;
---
>     if (ngDevMode || !(target instanceof EventTarget)) {
>       target[MONKEY_PATCH_KEY_NAME] = data;
>     }
7389a7393
>                             delete callContext[MONKEY_PATCH_KEY_NAME];delete callContext._ref;
7399a7404
>                         delete context[MONKEY_PATCH_KEY_NAME];delete context._ref;
7691a7697
>         delete rNode[MONKEY_PATCH_KEY_NAME];

and our patch , in npm post-install is to call this after the ngcc has executed:-

patch -N node_modules/@angular/core/bundles/core.umd.js patches/@angular+core+12.2.7.umd.patch || true
patch -N node_modules/@angular/core/fesm2015/core.js patches/@angular+core+12.2.7.fesm2015.patch || true

The downside is we have to keep track when we update Angular

[theSumit67]

For anybody looking for an ugly workaround, I've yet to see any issues with this patch of attachPatchData:

import { process as runNgcc } from '@angular/compiler-cli/ngcc';
import { existsSync, readFileSync, writeFileSync } from 'fs';
import { join } from 'path';

const ANGULAR_CORE_PATH = '@angular/core';
const ANGULAR_CORE_NGCC_PATH = `${ANGULAR_CORE_PATH}/__ivy_ngcc__/fesm2015/core.js`;

const REPLACE_AT = `
function attachPatchData(target, data) {
    ngDevMode && assertDefined(target, 'Target expected');
    target[MONKEY_PATCH_KEY_NAME] = data;
}
`.trim();

const REPLACE_WITH = `
function attachPatchData(target, data) {
    ngDevMode && assertDefined(target, 'Target expected');
    if (ngDevMode || !(target instanceof EventTarget)) {
      target[MONKEY_PATCH_KEY_NAME] = data;
    }
}
`.trim();

// https://github.com/angular/angular/issues/41047
export function patchAngularToRemoveNgContextFromDom(nodeModulesPath: string): void {
  const patchFilePath = join(nodeModulesPath, ANGULAR_CORE_NGCC_PATH);

  if (!existsSync(patchFilePath)) {
    runNgcc({
      basePath: nodeModulesPath,
      targetEntryPointPath: join(nodeModulesPath, ANGULAR_CORE_PATH),
      createNewEntryPointFormats: true,
      propertiesToConsider: ['fesm2015'],
    });

    if (!existsSync(patchFilePath)) {
      throw new Error(`Tried to patch @angular/core but could not find ${patchFilePath}.`);
    }
  }

  const fileContent = readFileSync(patchFilePath, 'utf8');
  const replacedFileContent = fileContent.replace(REPLACE_AT, REPLACE_WITH);

  if (!replacedFileContent.includes(REPLACE_WITH)) {
    throw new Error(`Could not patch memory leak in @angular/core.`);
  }

  if (fileContent.length === replacedFileContent.length) {
    return;
  }

  writeFileSync(patchFilePath, replacedFileContent);
}

You are a life saver! With a similar fix our app is now smooth and performant on even the slower iOs Air2s ( ours is a hybrid ionic Ng app ). Basically we have 100s of cards that can be paginated back and forth and the dom nodes and memory were climbing up. We added this "hack" and the UI is now much better than ever. Our diffs for the patch were

6296a6297
>     // testing FESM
6298c6299,6301
<     target[MONKEY_PATCH_KEY_NAME] = data;
---
>     if (ngDevMode || !(target instanceof EventTarget)) {
>       target[MONKEY_PATCH_KEY_NAME] = data;
>     }
7389a7393
>                             delete callContext[MONKEY_PATCH_KEY_NAME];delete callContext._ref;
7399a7404
>                         delete context[MONKEY_PATCH_KEY_NAME];delete context._ref;
7691a7697
>         delete rNode[MONKEY_PATCH_KEY_NAME];

and our patch , in npm post-install is to call this after the ngcc has executed:-

patch -N node_modules/@angular/core/bundles/core.umd.js patches/@angular+core+12.2.7.umd.patch || true
patch -N node_modules/@angular/core/fesm2015/core.js patches/@angular+core+12.2.7.fesm2015.patch || true

The downside is we have to keep track when we update Angular

We are using micro front-end architecture with ngx-build-plus and Angular v10.2.
One angular app is using another app as a web component exported using angular/element. We were facing a similar issue of high DOM Nodes getting added each time we refer the web component and those were not getting released when we were moving away from this page.

We tried the above mentioned approach & we got the benefit as the reference of WebComponent class is not leaking any memory. However, all of the DOM Nodes of this web component are getting detached( Detached HTMLElement/Div Element, etc) when the component is destroyed.

Can someone help with this? Thanks!

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}