Memory leak just including the module

[jessewilliams1]

node.js 10.10.0
npm 6.4.1
express: 4.17.1

Memory usage went from 60MB (normal) to 450MB over 9 hours by simply including the module:

const quillConverter = require('node-quill-converter');

[joelcolucci]

Hi @jessewilliams1 Thanks for submitting this. I'll look into it.

[jessewilliams1]

This looks suspiciously like an infinite loop if it were called:

document.execCommand = function (command, showUI, value) {
  try {
      return document.execCommand(command, showUI, value);
  } catch(e) {}
  return false;
};

[joelcolucci]

Hmmm. I'll test it out. Thanks for your patience.

[adesege]

Hey @joelcolucci, the leak is probably due to the JSDOM library and the fact that you are reading the quill file synchronously. That could have an impact when converting large files.

The reason why the leak occurs by just including the file is that the reading of the file is done at the top level. So whether you use the package or not, it will always read the files.

I'm using this package locally and how I solved it is by using fs.promises.readFile and instead of using require.resolve, I used path.join to get the full path to quill.min.js file.

The performance improved greatly.

[joelcolucci]

Thanks @adesege. I'll try out those adjustments. I appreciate your time and feedback.

[adesege]

You are welcome man! The package gave me a good start though. Thanks.

[glownesWlc]

@joelcolucci Is memory leak issue resolved now? I have to convert HTML to Delta on nodejs server side. So i need this package to implement on node side

[joelcolucci]

Hi all, thank you for your patience. I am dedicating some time this week to revisit this issue. Will update soon.

[joelcolucci]

Hi all! Just published version ^0.3.3 which improves memory usage and prevents the module from hanging on import.

It looks like Quill's setText method eats up a lot of memory. I'm going to continue to troubleshoot. Let me know if you all have any questions.

[extremerotary]

@joelcolucci
Thanks a million for looking into this! I'm going to get the version updated next week and submit my findings.

[extremerotary]

Unfortunately, in my testing, there is still a memory leak. I updated to version 0.3.3 and let the node server run on the server. It took about 2 days, but the memory usage kept increasing until it ran out of memory.

[joelcolucci]

@extremerotary Thanks for the feedback.

In my testing I noticed that a potential leak occurs when calling Quill.setContents. Quill might have a circular reference preventing the garbage collector from cleaning things up. I'm away this weekend but will dive back in next week.

Are you using this in a web application or an ETL pipeline?

[extremerotary]

@joelcolucci
Hey Joel,
We use it in a web application.

[Trunksome]

I experienced the same issue after including the module in a nodejs app. I use it to convert quill delta to HTML.

See memory usage in blue:

nodejs 14.9.0 npm 6.14.7 express 4.17.1 node-quill-converter 0.3.3

At the moment, I simply require the module like so: const { convertDeltaToHtml } = require("node-quill-converter"); and then use it when needed: convertDeltaToHtml(delta); Is there anything I should be doing differently?

[joelcolucci]

Hey @Trunksome, thanks for letting me know. I'm backed up work up at the moment. I'll try and spin up a long running process and see if I can replicate, isolate this issue.

[lencyforce]

Hi @joelcolucci really nice work on this project!
Is there any update on this issue? I'm using the module the same way as @Trunksome, and the service is eating up lots of memory and dies frequently.

[Trunksome]

I ended up closing this library and wrapping everything in a class (thereby only creating a JSDOM when needed) and calling

close() {
    this.cache = null;
    this.DOM.window.close(); // found it here: https://stackoverflow.com/questions/13893163/jsdom-and-node-js-leaking-memory
}

once i'm finished with the converter.
Memory leaks are gone.