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 md-to-react-email dependency (Markdown component) #1780

Open
maxbrereton opened this issue Nov 19, 2024 · 2 comments
Open

Memory leak in md-to-react-email dependency (Markdown component) #1780

maxbrereton opened this issue Nov 19, 2024 · 2 comments

Comments

@maxbrereton
Copy link

maxbrereton commented Nov 19, 2024

Describe the Bug

I have already raised a PR here (codeskills-dev/md-to-react-email#30) with a fix on the repo md-to-react-email which this repo uses for its Markdown component with a detailed explanation there. I will copy it here for better tracking and visibility. Also not sure if anyone looks at that other repo.

Hi,

We have been using react-email in production under fairly heavy load and have noticed a memory leak.

After some investigation I have narrowed it down to the repo md-to-react-email and specifically how the underlying marked library is being used currently.

I found this in marked library documentation: "Be careful: marked.use(...) should not be used in a loop or function. It should only be used directly after new Marked is created or marked is imported." (here: https://marked.js.org/using_advanced).

And the above warning from the marked documentation seems to be violated here in this repo: https://github.com/codeskills-dev/md-to-react-email/blob/master/src/parser.ts#L13

Here are some memory snapshots from a small test I ran (using clinic.js to analyze memory):
Before fix:
Screenshot 2024-11-18 at 15 47 35

After fix:
Screenshot 2024-11-18 at 15 47 42

You can see that the memory profile is flat after the changes have been made (before it would eventually just crash with OOM error).

I also took this screenshot from running our production code locally and load testing the application and you can see that it retains the marked.js instances and does not release them. Eventually these build up and cause the OOM errors:

Screenshot 2024-11-12 at 14 26 29

Which package is affected (leave empty if unsure)

No response

Link to the code that reproduces this issue

Have added demo project here that shows using react-email Markdown component creates memory leak: https://github.com/maxbrereton/react-email-memory-leak-demo

See PR here: codeskills-dev/md-to-react-email#30

To Reproduce

Use the Markdown component from react-email. To best see the memory leak you should create large amounts of Markdown components.

We initially produced this by using this in our normal production workload where it would cause OOM during high volume sends.

However, the graphs above are from a small test I wrote here - this was done from the context of the repo md-to-react-email where the memory leak occurs:

const { parseMarkdownToJSX } = require("./parseMarkdownToJSX");

const markdown = '## Lists'

async function testParse() {
  for (let i = 0; i < 800; i += 1) {
    for(let j = 0; j < 1000; j += 1) {
      parseMarkdownToJSX({ markdown });
    }
    await new Promise(r => setTimeout(r, 200));
    console.log('processed 1000, overall counter -> ', i);
  }
}

testParse()
  .then(() => {
    console.log('done');
  })
  .catch(err => {
    console.error('An error occurred:', err);
  });

Expected Behavior

When you use the Markdown component from react-email it should not cause a memory leak.

What's your node version? (if relevant)

No response

@gabrielmfern
Copy link
Collaborator

Thank you so much for reporting this here! I asked for the package owner to review, I left my approving review to help.

Once it is released there, I'll update our Markdown component.

@gabrielmfern
Copy link
Collaborator

Just released a canary that updated md-to-react-email to the version with your fix, check out @react-email/[email protected]/@react-email/[email protected].

Thank you so much for fixing and looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants