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 #359

Closed
songzhuang opened this issue Apr 5, 2018 · 12 comments
Closed

Memory Leak #359

songzhuang opened this issue Apr 5, 2018 · 12 comments

Comments

@songzhuang
Copy link

Hi,
I have developed a system with node.js, express with ejs as the rendering engine. The application locks up after about 30 minutes of run time. Using the Chrome debugger, looks like there is a memory leak when ejs does the rendering. eg. If I click on a link 5 times, the html for the 5 is not garbage collected.

How would I handle this?
Thanks,

@secreter
Copy link

mark.It's seams I met the same problem.

@mde
Copy link
Owner

mde commented Apr 21, 2018

EJS just creates a function out of your template string, then runs it with your data as input parameters. It's highly unlikely that this is a memleak issue in EJS itself. (EJS is also a pretty widely used module, so any memleak issues would be visible to tons of people.)

Since you're seeing this problem in the browser, it seems more likely that references to either the returned HTML, or to the created DOM node, are still hanging around, preventing that memory from getting GC'd.

If you could provide a very minimal test to demonstrate this, I might be able to help, but I'm pretty sure this is an issue with your application, not with EJS.

@lxcid
Copy link

lxcid commented May 14, 2018

Hmmm, I think I might have met this problem too.

@jackycute
Copy link

I used node --inspect app.js to run our server and use ApacheBench to test it.
Then I found that data in memory stack are mostly from the ejs rethrow related function.
Anyone have clues or this is normal?
2018-06-01 10 30 18

@mde
Copy link
Owner

mde commented Jun 2, 2018

What I'm seeing in the screenshot looks like a string variable for the source that the EJS Template object uses to compile into a runnable function. Depending on what's in your template text, this could be very large.

@mde
Copy link
Owner

mde commented Jun 2, 2018

Again, given how widely used EJS is, any memleak problem in EJS itself would be immediately visible to a huge group of people. If someone can provide a minimal test demonstrating a memleak in EJS, I'm happy to take a look, but I'm going to close this issue for now since it's mostly people talking vaguely about memleak issues in their apps, and nothing specific to EJS. We can certainly reopen if there's any evidence of a real memleak issue in EJS.

@mde mde closed this as completed Jun 2, 2018
@magic890
Copy link

magic890 commented Jun 4, 2018

Same problem here, with the string and concatenated string. Seems that they are not going to be collected by GC.

@songzhuang @lxcid @secreter @jackycute @mde did you find any solution or workaround?

@magic890
Copy link

magic890 commented Jun 14, 2018

I figure out that if you run your app inside a container you can limit to 256 MB the memory of the container to force the GC to clean up the unused references. In this way I fixed our situation.

I'm also using EJS forcing with the option strict: true to avoid possible global variable pollution.

I tried to limit the memory management on node following this nodejs/node#7937 using --max_old_space_size=265 as V8 option but it does not seems working.

Here is how it changed the memory allocation after the limitation on container level:

memorymanagement

@Starystars67
Copy link

Starystars67 commented Jul 4, 2018

Dear @mde i have encountered this issue too here is a copy of code that i have used to minimize the external variables, ignore the moment library its used for the ejs page i was loading from my project.

const memwatch   = require('memwatch-next');
var moment = require('moment');

// load the things we need
var express = require('express');
var app = express();

// set the view engine to ejs
app.set('view engine', 'ejs');

// use res.render to load up an ejs view file

// index page
app.get('/', function(req, res) {
	res.render('public/landingpage', {moment: moment});
});

app.listen(8080);
console.log('8080 is the magic port');

memwatch.on('leak', (info) => {
  console.log(info)
});

@youneskasri
Copy link

I figure out that if you run your app inside a container you can limit to 256 MB the memory of the container to force the GC to clean up the unused references. In this way I fixed our situation.

I'm also using EJS forcing with the option strict: true to avoid possible global variable pollution.

I tried to limit the memory management on node following this nodejs/node#7937 using --max_old_space_size=265 as V8 option but it does not seems working.

Here is how it changed the memory allocation after the limitation on container level:

memorymanagement

Can you tell me more about your solution i am facing the same issue

@magic890
Copy link

@YounesTheGreat that app is run inside a Docker container on Kubernetes.
Limit the memory of the Docker container to something small, like 256 MB, to force the garbage collector of NodeJS to start and free unused memory.
The strict: true option does not seems to make any change.

@mde
Copy link
Owner

mde commented Sep 23, 2018

@YounesTheGreat What we would need is a minimal example that uses only EJS, not multiple other libs including Express, and other unknown application code (e.g., public/landingpage). Your app uses EJS, and your app has a memleak. That does not mean that EJS has a memleak.

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

8 participants