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

"Warning: Possible EventEmitter memory leak detected" occured when define many models #112

Closed
ma-hiro opened this issue Aug 4, 2021 · 6 comments · Fixed by #115
Closed

Comments

@ma-hiro
Copy link

ma-hiro commented Aug 4, 2021

First, thanks for this awesome library. I'm using this library in my project and it's very useful.

I have defined a number of models using factory(), but when it exceeds 11, a warning occurs in chrome.
Is there a way to prevent this?

Describe

If I define 11 or more models using factory(), the following warning will be displayed.

events.js:46 MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 delete listeners added.
Use emitter.setMaxListeners() to increase limit

This is displayed when you define the model, and even more when you call model.create().
(As for the event, only a warning is displayed, and the mock function works normally)

Example project

https://github.com/ma-hiro/sample-msw-data

When I launch the app and access http://localhost:3000, I get a lot of warnings.

This code is the cause of the warning.
If I try removing some models, the warning disappears.

It can be suppressed by setting the EventEmitter's maxListener, but the EventEmmiter in this library is probably cannot be referenced externally.
Therefore, we are tentatively dealing with the following.

import { EventEmitter } from 'events';
EventEmitter.defaultMaxListeners = 11; // Number of models you want to define or 0(infinity)

However, since this setting affects all EventEmitters, I would like to use setMaxListeners() to set it only for specific Emitters.

@juusot
Copy link

juusot commented Aug 12, 2021

@ma-hiro I can confirm. I experienced the same issue in my project.

I tried different ways of going around this problem, but I couldn't find a solution.

@kettanaito
Copy link
Member

Hey, @ma-hiro. Thank you for reporting this!

I'll look into the reproduction repository you've posted and update this thread. I don't have any ETA as to when this is going to be fixed. Anybody is welcome to investigate alongside me, as it's most likely an issue of inefficiently added event listeners or a missing listeners cleanup.

@JoseRFelix
Copy link

JoseRFelix commented Aug 13, 2021

@juusot I encountered this same issue. Since it only occurs when instantiating 10 Event Emitter listeners, we can avoid this by creating a factory for every 10 schemas.

I created a quick abstraction for this:

import { factory } from "@mswjs/data"
import { FactoryAPI } from "@mswjs/data/lib/glossary"

schemas = { /* Your schemas go here */ }

function chunkArray<T>(array: T[], size: number): T[][] {
  const result = []
  const arrayCopy = [...array]

  while (arrayCopy.length > 0) {
    result.push(arrayCopy.splice(0, size))
  }

  return result
}

function getTestDb() {
  const FACTORY_CHUNK_SIZE = 10
  const INITIAL_DB = {}

  // For each ten items add a new factory. This is required to avoid browser event emitter limits
  const chunkedSchemas = chunkArray(Object.entries(schemas), FACTORY_CHUNK_SIZE)

  const db = chunkedSchemas.reduce((acc, schema) => {
    const dbFactory = factory(Object.fromEntries(schema))
    return { ...acc, ...dbFactory }
  }, INITIAL_DB as FactoryAPI<typeof schemas>)

  return db
}

@kettanaito
Copy link
Member

kettanaito commented Aug 17, 2021

Investigation

  • The issue appears only in a browser.
  • The issue is caused by the sync extension that synchronizes entity changes across multiple open tabs.
  • The issue is caused by the listeners attached to the db.events emitter:

db.events.on('create', broadcastDatabaseEvent('create'))
db.events.on('update', broadcastDatabaseEvent('update'))
db.events.on('delete', broadcastDatabaseEvent('delete'))

I believe there is no issue per se, just that the event emitter finds it suspicious that 33 listeners were added to it (11 for each event: create, update, delete) and warns us that may be the issue.

I'll look into the logic, perhaps we can optimize it, or perhaps the solution is to set the maximum number of listeners on the db.events since the number of models is fixed and cannot be changed after the factory() call.

There's certainly room for optimization, as there may be 3 listeners total added to the entire Database instance to handle all create/update/delete operations, instead of attaching listeners to each model.

@kettanaito
Copy link
Member

The issue has been resolved in #115. Published in 0.5.1. Please update and let me know that it's gone. Thanks.

@ma-hiro
Copy link
Author

ma-hiro commented Aug 18, 2021

Updating to 0.5.1 solved my problem.
Thank you for your quick and awesome support!

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.

4 participants