Bug: memory leak with when using Http(s)Agent keepAlive=true about mappersmith HOT 5 CLOSED

Hi!
We were hit by this memory leak in production🙃, I wish we saw this issue earlier. (greatly reported btw).
I've taken the liberty to write a spec to reproduce the issue and apply the fix. I will open a pull request tomorrow morning if that's fine with both of you - we reaaaally need this fixed urgently.

edit: @klippx I created the PR with the spec and the fix

from mappersmith.

This has been released in 2.43.0!

from mappersmith.

No response needed

Adding a comment here for future readers to understand the potential impact of this bug.

Example App Impact

• App uses mappersmith indirectly via @kafkajs/confluent-schema-registry for parsing Kafka messages
• App is deployed on AWS Lambda
• App has global http / https agent override set to enable keep-alive reuse of sockets (off by default in Node.js for requests made to other services)
• Nominally, the app would process 1k Kafka messages in 5 seconds or less
• When processing infrequent events, allowing the HTTP socket to close, the issue was not apparent
• When processing a backlog of millions of messages, resulting in continuous invokes, keeping the socket open for many minutes, the runtimes would increase from 5 seconds to 500 seconds until the Lamba eventually timed out
  • Note that in this case the payloads were not larger, they simply were invoked one after the other without allowing the Lambda to be idle long enough for the socket that mappersmith owned to close

This application saw a 100x increase in runtime due to the accumulation of event emitters on these long-lived sockets.

Thank you so much for the fix!

from mappersmith.

Thanks for reporting this, really well written issue.

Is there any chance that you would like to try to take your level of contribution to the next level and create a branch containing a spec that reproduces the issue? 🙏

It would be super appreciated, especially in case you already have a battle tested fix on your end.

from mappersmith.

Hi @klippx!

In an odd twist of fate - I did actually start out with the idea of presenting this issues in form of a pull-request.

Unfortunately - and that it completely on me - the setup of the test cases for this package uses structure that I'm not familiar with (at least I was having trouble detecting a clear setup-execute-assert pattern that I'm accustomed to).

I also had trouble getting the project to install properly (where it got stuck with something with pre-install scripts).

As such: I decided that giving a clear report on the issue and the serious nature of it might be enough for the author(s) (or people more in-tune with the setup of the particular project) to be motivated to quickly tackle the issue themselves.

from mappersmith.