Skip to content

test: replicate memory leak (#79)#81

Open
gajus wants to merge 2 commits intomainfrom
gajus/debug-memory-leak
Open

test: replicate memory leak (#79)#81
gajus wants to merge 2 commits intomainfrom
gajus/debug-memory-leak

Conversation

@gajus
Copy link
Copy Markdown
Owner

@gajus gajus commented Sep 27, 2023

This recreates the issue #79

Thank you to @ferrata for helping to create a replication of the issue https://github.com/ferrata/slonik-jest-memory-leak-demo

@gajus
Copy link
Copy Markdown
Owner Author

gajus commented Sep 27, 2023

Removing binding on the event emitter eliminates the error.

import { type LogWriter } from '../types';

const createBlockingWriter = (stream: NodeJS.WritableStream): LogWriter => {
  return (message: string) => {
    stream.write(message + '\n');
  };
};

export const createNodeWriter = (): LogWriter => {
  // eslint-disable-next-line node/no-process-env
  const targetStream = (process.env.ROARR_STREAM ?? 'STDOUT').toUpperCase();

  const stream =
    targetStream.toUpperCase() === 'STDOUT' ? process.stdout : process.stderr;

-  stream.on('error', (error) => {
-    if (error.code === 'EPIPE') {
-      return;
-    }
-
-    throw error;
-  });

  return createBlockingWriter(stream);
};

At a glance, I am not 100% sure why this is reported as a memory leak.

@gajus
Copy link
Copy Markdown
Owner Author

gajus commented Sep 27, 2023

@vakrilov
Copy link
Copy Markdown

vakrilov commented Mar 6, 2024

I hit a similar (probably the same) issue resulting in the following warning during test execution:

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [WriteStream]. Use emitter.setMaxListeners() to increase limit

Explanation

For each test module(file) Jest creates a new environment. Each environment has a clean global object, so when is loaded roarr it goes trough the createRoarrInitialGlobalState and adds another stream.on('error', ...) listener. When those listeners add up to 10 the ☝️ error is thrown when trying to add another one.

Good news is that the error is try/catch-ed, so it does not effect the test execution. That said, it would be best if there was a way to cleanup the stream.on('error', ...) listener. This will enable us to use the jest environment's teardown() hook to do the cleanup and the both memory leak and max listeners warnings would be avoided.

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 this pull request may close these issues.

2 participants