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

Possible context memory leak? #868

Closed
validkeys opened this issue Oct 25, 2018 · 4 comments
Labels
bug
Milestone

Comments

@validkeys
Copy link
Contributor

@validkeys validkeys commented Oct 25, 2018

Hi Guys,

I was attempting to debug a growing memory leak in my application and think I know where it is. Currently, I use the beforeEach method to create one or more sequelize models and persist them to by DB. To ensure I can reference these models from each test, i do essentially the following:

beforeEach(async ({context}) => {
  let cache = {}
  cache.user = await Factory.create('user')
  context.cache = cache
})

After a while I noticed that none of these models were being garbage collected after running many tests.

In runner.js, each test is being pushed into the reporters array:

state.report.tests.push(test);

...And therefore holding on to all of the variables I created in my context until all tests have run. Since the flags given to each test is a clone, I can never nullify the content of my original test. So as each of my tests get pushed into the tests array above, memory usage grows.

I can't nullify that context in my afterEach hooks because I'm not getting the original context:

const flags = { notes: [], context: item.context || state.currentContext };

Perhaps context was not meant to be used for large objects such as data models, but I would think that once a test is run, it's context is no longer necessary. Unless I'm missing something, I'm happy to push a PR that nullifies the test's context once it's been run.

Let me know. Thanks!

@geek geek added the bug label Oct 25, 2018
@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Oct 25, 2018

Deleting the context from the test after the test is complete should be safe and I'll accept a PR to fix this. If you don't get around to it, I'll try to fix this up within the next week. Thanks for reporting this bug.

@validkeys

This comment has been minimized.

Copy link
Contributor Author

@validkeys validkeys commented Oct 25, 2018

@geek I've created a PR here to address the above concerns: #869

@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Oct 25, 2018

@validkeys This is now published as 17.0.2, thanks for the help

@validkeys

This comment has been minimized.

Copy link
Contributor Author

@validkeys validkeys commented Oct 25, 2018

You got it. Thanks for the quick response time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.