Fix memory leak

[bhunjadi]

Memory fix implementation based on findings and discussion in issue #378.
This should not be seen as a definite solution, but it should be a place to start.

Each node has a cleanup function which performs two things:

1. calls unsetMeasureFunc - this solves the problem with leaked instances (Text, Image)
2. calls Yoga.Node.destroy(this) - after testing export of a larger document with 1000 images with the same src I always got the error "Cannot enlarge memory arrays" and adding Yoga.Node.destroy solved this error. This is something that happened in our app after generating around 70 pdfs (server side) with size of 15-20 pages.

So it is possible this might fix #314. Also fixes #590

Implementation

Idea is that on each remove() called from wrapping algorithm we clean the removed child.
And after rendering, root instance calls cleanup() on the children:

async render() {
    this.instance = new PDFDocument({ autoFirstPage: false });
    await this.document.render();
    **this.cleanup();**
    this.isDirty = false;
}

[n6g7]

Looks good, I'll test this on my code base!
Also, not sure you meant to change the mode of all of those files (644 to 755).

[bhunjadi]

Argh, thanks. Fixed now.
Looks like permissions were changed because of samba mount.

[diegomura]

Awesome! Thank you very much! @n6g7 if you can test it that would be great. I'll try to test it today also 😄

[n6g7]

So this PR does fix the leak in the reproduction gist but not in my original project. 🤔
I might be overlooking something, I'll look into it more tomorrow.

[bhunjadi]

I tried with your project and to me it looks like it is ok.
I used yarn dev, generated 1 pdf (/demo), made a heapdump, then generated another 10 docs and then made a heapdump again. I see no leaks there regarding leaked Root, Text, View etc.

Could you post your heapdumps when you do the tests again? Thanks

[n6g7]

Yep you're right, it looks like it's working! :D
I actually was still using @react-pdf/node instead @react-pdf/renderer for some reason so was
running unpatched code unknowingly.

[diegomura]

I’m going to test it between today and tomorrow and then merge it! Thanks for taking care of this 🎉

[diegomura]

So, I tried this today (sorry for taking that much!) and I found issues when the document re renders based on a changing state:

This is a very simple app that renders an image and a counter (that increases when clicking the button). First renders work, but afterwards it doesn't and it breaks the layout process.

What we should do is only cleaning the subpages nodes, not the children since these are reused on each render to calculate the next subpages. I tried this approach and fixed this. I'll push it here. Could you please try if the gist and your projects to see that the memory leak is still gone?

[n6g7]

Hey @diegomura, I applied your commit on both the gist and my project and the leak was back. :(

[diegomura]

@n6g7 got it. We should definitely cleanup children in node (since those nodes are also used once in that env), but not on the web. Will appreciate a lot if you can test again in the gist and your project!

[bhunjadi]

Looks to me that with latest commit everything is OK - no memory leak in the gist nor my project!

[n6g7]

Yes, same results here, no leak in gist or my project with this latest commit. :)
Well done!

[diegomura]

Thank you! Merging this now and I’ll publish a new version later