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 ? #27

Closed
m4nuC opened this issue Mar 8, 2024 · 15 comments
Closed

Memory leak ? #27

m4nuC opened this issue Mar 8, 2024 · 15 comments

Comments

@m4nuC
Copy link

m4nuC commented Mar 8, 2024

When running an application that generate a lot of image, the memory seem to be steadily increasing, after some point this error show up.

Can there be a memory leak somewhere ?

function getImports() {

 const imports = {};

 imports.wbg = {};

imports.wbg.__wbg_new_15d3966e9981a196 = function (arg0, arg1) {

  const ret = new Error(getStringFromWasm0(arg0, arg1));

^

error: SVG data parsing failed cause unknown token at 1:1

at /app/node_modules/@wevm/vercel-og/dist/index.node.js:26285:17

at WASM  ([wasm code])

at WASM  ([wasm code])

at new Resvg (/app/node_modules/@wevm/vercel-og/dist/index.node.js:26154:7)

at new Resvg2 (/app/node_modules/@wevm/vercel-og/dist/index.node.js:26396:5)

at /app/node_modules/@wevm/vercel-og/dist/index.node.js:26646:19
@m4nuC
Copy link
Author

m4nuC commented Mar 8, 2024

After more testing, it seems related to emojis. Underload test I confirm that external memory grows and never get garbage collected

@jxom
Copy link
Member

jxom commented Mar 8, 2024

Using Bun? Could be Bun related but will need to investigate.

@m4nuC
Copy link
Author

m4nuC commented Mar 9, 2024

Yes it was using bun. Reproduction is pretty simple.

  • Send a bunch of POST queries on an /image route that includes an emoji
  • Memory steadily goes up till eventually that error show up
  • Removing the emoji fixes the issue

@m4nuC
Copy link
Author

m4nuC commented Mar 10, 2024

This also happens in nodejs.
Can't use this on any amount of traffic right now. Memory eventually grows up to 4gig then app breaks

@jxom
Copy link
Member

jxom commented Mar 10, 2024

Not sure if this is an issue with hono-og then, but maybe @vercel/og.

@m4nuC
Copy link
Author

m4nuC commented Mar 10, 2024

Could be, in which case this is bad.

Seems strange though, I assume that package got exposed to quite some load

@m4nuC
Copy link
Author

m4nuC commented Mar 10, 2024

What does @wevm/vercel-og dependency do ? is it just the build of @vercel/og. No modifications are made to it ?

@jxom
Copy link
Member

jxom commented Mar 10, 2024

That’s only for the Bun output, Bun couldn’t handle loading external WASM files, so had to inline them.

@m4nuC
Copy link
Author

m4nuC commented Mar 10, 2024

@jxom so good new is that it doesn't look to be @vercel/og. I've tried to render image from a route using Satori directly and I can confirm that the memory does get garbage collected, so the leak is somewhere in hono-og

@jxom
Copy link
Member

jxom commented Mar 10, 2024

Didn’t you say it happens on Node.js though? Node.js uses the @vercel/og entry. @vercel/og also isn’t just Satori, it also uses Resvg too which I would think would be the culprit as it uses WASM.

@m4nuC
Copy link
Author

m4nuC commented Mar 10, 2024

Satori + Resvg works fine on both node and bun

@jxom
Copy link
Member

jxom commented Mar 10, 2024

Not sure where the memory leak would arise then as this library is a simple wrapper over @vercel/og with basic Hono JSX -> React JSX conversion.

As always, a minimal reproducible example would help a lot so I can investigate further.

@m4nuC
Copy link
Author

m4nuC commented Mar 11, 2024

To reproduce.

Spin up a new app. I've tried Node and Bun.

Take any image path, eg: http://localhost:3000/image?image=.....

Add a memory logger middleware

  app.use("*", async (c, next) => {
    console.log(process.memoryUsage().external)
    await next();
  })

Run an https://www.artillery.io load test on that path

Notice the memory grows linearly with the number of calls and never gets garbage collected

@m4nuC
Copy link
Author

m4nuC commented Mar 11, 2024

If you are saying that a NodeJS setup doesn't use the wrapped Vercel-og then it could be on Vercel-og side. They do some some dynamic font loading that may be the issue. However I can confirm that satori + revsg on their own do no have this issue

@jxom
Copy link
Member

jxom commented Mar 13, 2024

Fixed via #28.

@jxom jxom closed this as completed Mar 13, 2024
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

2 participants