Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak due to objects created in storageFor not being destroyed. #376

Closed
Mikek2252 opened this issue Feb 2, 2024 · 2 comments · Fixed by #377
Closed

Memory leak due to objects created in storageFor not being destroyed. #376

Mikek2252 opened this issue Feb 2, 2024 · 2 comments · Fixed by #377

Comments

@Mikek2252
Copy link
Contributor

The objects created in the storageFor helper and then stored in storages are not being destroyed after each test instance. This means the event listener is not removed (as willDestroy does not trigger) and is leaking the app container.

This can we seen by running the test suite of this addon and taking a memory snapshot.

@fsmanuel
Copy link
Member

fsmanuel commented Feb 2, 2024

@Mikek2252 thanks for reporting! Do you use the Test Helpers:

import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { visit, currentURL } from '@ember/test-helpers';
import resetStorages from 'ember-local-storage/test-support/reset-storage';

module('basic acceptance test', function(hooks) {
  let hooks = setupApplicationTest(hooks);

  hooks.afterEach(function() {
    if (window.localStorage) {
      window.localStorage.clear();
    }
    if (window.sessionStorage) {
      window.sessionStorage.clear();
    }
    resetStorages();
  });

  test('can visit /', async function(assert) {
    await visit('/');
    assert.strictEqual(currentURL(), '/');
  });
});

If this does not fix the memory leak I'll need to investigate.

@Mikek2252
Copy link
Contributor Author

Mikek2252 commented Feb 2, 2024

@fsmanuel Hi, yeah i am using resetStorages but the objects inside of storages are not destroyed. I think this is because the Object returned by storageFor is wrapped in a computed property so ember doesn't know it needs to destroy it. I have created a PR with a fix. You should be able to see the leak if you run the tests of ember-local-storage and do a snapshot. Also for clarification the leak only occurs on objects created by storageFor and the objects created in the adapter are destroyed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants