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 in knex v0.13.0 #2165

Closed
badwulfy opened this issue Jul 20, 2017 · 9 comments
Closed

Memory leak in knex v0.13.0 #2165

badwulfy opened this issue Jul 20, 2017 · 9 comments

Comments

@badwulfy
Copy link

badwulfy commented Jul 20, 2017

Hi,

When importing, then destroying, then reset modules a lot of times with jest, I have some warning about memory leak MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGABRT listeners added. Use emitter.setMaxListeners() to increase limit

I think that destroy() leave listener that should be removed.

Here is the (useless) code to reproduce the issue :

//@flow

import k from 'knex';
import path from 'path';

const config = {
  client: 'sqlite3',
  connection: {
    filename: path.join(__dirname, './database.sqlite'),
  },
  pool: {
    min: 1,
    max: 1,
  },
  migrations: {
    tableName: 'knex_migrations',
  },
  useNullAsDefault: true,
  timezone: 'UTC',
};

let knex;

describe('pppppppppp', () => {
  beforeEach(async () => {
    jest.resetModules();
    knex = k(config);
  });

  afterEach(async () => {
    await knex.destroy();
  });

  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
  test('test', () => {});
});

the output :

Is it a bad use or a bug ?
Thanks in advance.

@badwulfy
Copy link
Author

it seems to be the same issue as #2106

@elhigu
Copy link
Member

elhigu commented Jul 24, 2017

Thanks for report, would be great to add this kind of test (and fixes ofc) to test suite.

@wubzz
Copy link
Member

wubzz commented Jul 25, 2017

Can be reproduced as long as jest.resetModules() is called. I tested this example against branch of #2128 and there the warning no longer exists. So presumably that PR fixes the memory leak by upgrading generic-pool version.

@wubzz wubzz added the pool label Jul 25, 2017
@fl0w
Copy link

fl0w commented Jul 27, 2017

I've experienced this when using jest specifically myself. I'm wondering if this simply isn't a knex issue, but rather a jest issue. I do not experience equivalent issues in a project using mocha (while completely different projects, the setup is quite similar, both testing API endpoints using supertest/koa2/objection)

@fl0w
Copy link

fl0w commented Jul 27, 2017

@kiralex if you explicitly call jest.clearAllTimers() in afterEach, does it fix it for you?

@badwulfy
Copy link
Author

badwulfy commented Jul 27, 2017 via email

@badwulfy
Copy link
Author

@fl0w nop, it does not fix the issue

@elhigu
Copy link
Member

elhigu commented Jul 28, 2017

@kiralex Does it leak when running initialization and destroy sequence without jest?

@wubzz
Copy link
Member

wubzz commented Nov 1, 2017

From my previous tests this should be fixed by #2208, so closing this for now. If it's still an issue in upcoming 0.14, feel free to reopen.

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

No branches or pull requests

4 participants