Potential memory leak?

[satanTime]

Discussed in #5435

^{Originally posted by maxs-rose April 12, 2023}
After running ~2K tests the JS heap reaches 1GB of memory usage and no longer goes down.

There appears to be a lot of detached dom nodes handing around that seem to be related to ngMocksUniverse

Most of the test are setup similarly to the following:

describe("some component", () => {
     ngMocks.faster();

     const someServiceMock = MockService(SomeService);

     beforeAll(() => {
         return MockBuilder(SomeComponent).mock(SomeService, someServiceMock);
     );
     
    it("should do something", () => {
         const component = MockRender(SomeComponent).point.componentInstance;
         
         component.foo();

         expect(someServiceMock.serviceMethod).toHaveBeenCalled();
    });

    /* some more it blocks */
);

We have tried removing ngMocks.faster but that doesn't seem to have any affect.

Is this behavior due to how we are using NgMocks?

[satanTime]

Hi @maxs-rose,

I've tried to reproduce, but couldn't get the same behavior.

What I've seen was - during the test with and without ng-mocks memory spikes, and after all tests it drops back.
Also, memory snapshot doesn't show any deteched HTMLElements.

My test is:

import { Injectable } from '@angular/core';
import { Pipe, PipeTransform } from '@angular/core';
import { Directive } from '@angular/core';
import { ChangeDetectionStrategy, Component } from '@angular/core';
import { NgModule } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

import { MockBuilder, MockRender } from 'ng-mocks';
import {TestBed} from "@angular/core/testing";

@Component({
  selector: 'app-root',
  template: `
    <div *ngFor="let index of data">ng-mocks:{{ title }} {{ index }}</div>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush,
})
class AppComponent {
  public readonly title: string = 'hello';

  public readonly data: Array<string> = [];

  constructor() {
    for (let i = 0; i < 5000; i += 1) {
      this.data.push(`${i}`);
    }
  }
}

@Directive({
  selector: 'app-root',
})
class AppDirective {}

@Pipe({
  name: 'app',
})
class AppPipe implements PipeTransform {
  transform(): string {
    return this.constructor.name;
  }
}

@Injectable()
class AppService {}

@NgModule({
  imports: [BrowserModule],
  declarations: [AppComponent, AppDirective, AppPipe],
  providers: [AppService],
  bootstrap: [AppComponent],
  exports: [AppComponent],
})
class AppModule {}


describe('AppComponent', () => {
  beforeEach(() => MockBuilder(AppComponent, AppModule));
  // beforeEach(() => TestBed.configureTestingModule({
  //   imports: [AppModule],
  // }).compileComponents());

  for (let i = 0; i < 5000; i += 1) {
    it(`should create the app #${i}`, () => {
      const fixture = MockRender(AppComponent);
      // const fixture = TestBed.createComponent(AppComponent);
      // fixture.detectChanges()
      expect(fixture.componentInstance).toBeTruthy();
    });
  }
});

[maxs-rose]

Thanks for looking into this.

Having done some more investivation the majority of the detached elements seem to be comming from a pipe that is being injected into a service. This service is then being injected into a component and passed into a class.

Since this is in my companies app I cannot just give the code over but will attepmt to re-create it in a test project.

The basic layout is as follows:

// Allows us to change the currency symbol of the application based on the server configuration
@Pipe({
  name: 'dynamicCurrency',
  pure: false,
})
class DynamicCurrencyPipe implements OnDestroy, PipeTransform {
   constructor(
    @Inject(LOCALE_ID) private locale: string,
    @Inject(CURRENT_CURRENCY_SYMBOL) private currentCurrencySymbol: Observable<string>,
    private currencyPipe: CurrencyPipe,
    ref: ChangeDetectorRef
  ) {
    this.#detetorRef = ref;
  }

  // Destroy will clear any sybscriptions to CURRENT_CURRENCY_SYMBOL
}

// Pipe is declared in a shared module and CURRENT_CURRENCY_SYMBOL is declared as a BehaviourSubject in the main AppModule
@NgModule({
     declarations: [DynamicCurrencyPipe],
     exports: [DynamicCurrencyPipe],
     providers: [CurrencyPipe, DynamicCurrencyPipe],
})
class SharedModule {}

// Serving importing the pipe
@Injectable({
  providedIn: 'root',
})
class UtilService {
    constructor(private currencyPipe: DynamicCurrencyPipe) {}

    public formatNumber(number: FormatType, num?: number) {
        switch(type) {
             case: 'CURRENCY'
                   return this.currencyPipe.transform(num);
             // Other formatters
        }
}

// Component using the service is declared in a lazy loaded AdminModule that imports the SharedModule
@Component({ ... })
class Component {
    constructor(private utilService: UtilService) {
          this.data = new Data(utilService);
    }
}

// Test setup
describe('component', () => {
    beforeAll(() => {
        return MockBuilder(Component , AdminModule)
          .keep(UtilService);
    });

    it('should load', () => {
         const fixture = MockRender(ActionOverrideConfigurationComponent);

         expect(fixture.point.componentInstance).toBeTruthy();
    });

    it('should load 2', () => {
        // Important for there to be 2 MockRenders otherwise the deteached nodes do not appear
         const fixture = MockRender(ActionOverrideConfigurationComponent);

         expect(fixture.point.componentInstance).toBeTruthy();
    });
})

This seems to end up with detached DynamicCurrencyPipe objects that reference the service

I have manged to reliably cause this to happen in a single test (there are many like it) so will see if the prblem persists after rewriting this without using ng mocks

[maxs-rose]

@satanTime I think I have currently got the problem as narrow as I can get it and will see if I can work on an example repo to share with you but for now with the same setup as above with the services etc:

The following code will cause detached object nodes of the DynamicCurrencyPipe

describe('SomeTestComponent', () => {
  ngMocks.faster();

  beforeEach(() => {
    return MockBuilder(SomeTestComponent, AdminModule)
      .keep(Service1)
      .keep(Service2)
      .keep(UtilService);
  });

  it('should create 1', () => {
    const fixture = MockRender(SomeTestComponent);
    expect(fixture.componentInstance).toBeTruthy();
  });

  it('should create 2', () => {
    const fixture = MockRender(SomeTestComponent);
    expect(fixture.componentInstance).toBeTruthy();
  });

The following will NOT cause detached nodes:

describe('SomeTestComponent', () => {
  beforeEach(() => {
    return MockBuilder(SomeTestComponent, AdminModule)
      .keep(Service1)
      .keep(Service2)
      .keep(UtilService);
  });

  it('should create 1', () => {
    const fixture = MockRender(SomeTestComponent);
    expect(fixture.componentInstance).toBeTruthy();
  });

  it('should create 2', () => {
    const fixture = MockRender(SomeTestComponent);
    expect(fixture.componentInstance).toBeTruthy();
  });

Replacing .keep with .provide does not cause detached nodes with or without ngMocks.faster() and I cannot cause this to happen with a built in Angular testing.

Using beforeAll or beforeEach to declare the MockBuilder also have the same results.

So this looks like it is some interaction between .keep and ngMocks.faster().

Neither Service1 or Service2 have any injected dependencies and adding some dosnt seem to cause the issue either.

Hopefully this is good info for you to work off and again I will attempt to recreate this in a public repo for you :)

[maxs-rose]

I have managed to recreated the issue here https://github.com/maxs-rose/ng-mocks-leak-example.

It looks like ngOnDestroy isnt getting called on the UtilService so the subscription to the window is still active which is causing the detached nodes.

Of course in normal production code this isnt going to be a problem since the service is provided in root making the service a singlelton so (as far as i understand) we are never going to have it destroyed unless the window is closed. However, during unit tests this will ofc be different.

I belive it requires two it blocks to produce the detached node as you dont remove the component from the DOM after the last test

[satanTime]

Thank you for the example, now I can see the issue and was able to get detached elements.

import { CurrencyPipe } from '@angular/common';
import {
  Component,
  Inject,
  Injectable,
  InjectionToken,
  NgModule,
  OnDestroy,
  Pipe,
  PipeTransform,
} from '@angular/core';
import { MockBuilder, MockRender, ngMocks } from 'ng-mocks';
import {
  BehaviorSubject,
  Observable,
  Subject,
  fromEvent,
  takeUntil,
} from 'rxjs';

export const WINDOW = new InjectionToken<Window>('Window', {
  factory: () => window,
});

@Pipe({
  name: 'dynamicCurrency',
  pure: false,
})
class DynamicCurrencyPipe implements PipeTransform {
  transform(value?: number | string | null): string | null {
    return `£{value}`;
  }
}

@Injectable({
  providedIn: 'root',
})
class UtilService implements OnDestroy {
  #stop$ = new Subject<boolean>();
  #currentPressedKey = new BehaviorSubject<KeyboardEvent | undefined>(
    undefined
  );

  constructor(
    @Inject(WINDOW) window: Window,
    private currencyPipe: DynamicCurrencyPipe
  ) {
    if (
      typeof window?.addEventListener === 'function' &&
      typeof window?.removeEventListener === 'function'
    ) {
      fromEvent<KeyboardEvent>(window, 'keydown')
        .pipe(takeUntil(this.#stop$))
        .subscribe((res) => {
          this.#currentPressedKey.next(res);
        });

      fromEvent<KeyboardEvent>(window, 'keyup')
        .pipe(takeUntil(this.#stop$))
        .subscribe(() => {
          this.#currentPressedKey.next(undefined);
        });
    }
  }

  ngOnDestroy(): void {
    this.#stop$.next(true);
  }

  public test() {
    return this.currencyPipe.transform(2);
  }
}

@Component({
  selector: 'app-test',
  template: '<div>Component {{ test() }}</div>',
})
class TestComponent {
  constructor(private utilService: UtilService) {}

  test() {
    return this.utilService.test();
  }
}

@NgModule({
  declarations: [DynamicCurrencyPipe],
  exports: [DynamicCurrencyPipe],
  providers: [CurrencyPipe, DynamicCurrencyPipe],
})
class SharedModule {}

@NgModule({
  declarations: [TestComponent],
  imports: [SharedModule],
})
class AdminModule {}

describe('Will created detached nodes', () => {
  ngMocks.faster();

  beforeEach(() => {
    return MockBuilder(TestComponent, AdminModule).keep(UtilService);
  });

  it('should create 1', () => {
    const fixture = MockRender(TestComponent);

    expect(fixture.componentInstance).toBeTruthy();
  });

  it('should create 2', () => {
    const fixture = MockRender(TestComponent);

    expect(fixture.componentInstance).toBeTruthy();
  });
});

I'll check what ngMocks.faster should trigger to cause a normal destroy.

[maxs-rose]

Thats great thanks :)

At least we can get around this in our project by adding a default mock of the WINDOW injection token. Since the tests are being run in parallel registering event listeners on the real window object feels like it could cause some odd behaviour if we have tests that check for an events being fired 🤔 so it may end up being a permanent change to add a default mock of WINDOW for us at least.