[SSR] Controllers are not garbage collected correctly

[RafalFilipek]

Bug report

Describe the bug

Hi,

I found this bug in my Next.js app, but we think it's not related to NextJS itself but rather SSR in general.

Let's go straight to example:

To Reproduce

• clone this repo - https://github.com/RafalFilipek/nextjs-react-spring-bug
• yarn install
• yarn build
• yarn start

This command will start production version of application with --inspect flag to enable NodeJs debugger.

• open [http://localhost:3000](http://localhost:3000) in Chome
• open DevTools and click NodeJS logo on the left

This will open Dedicated NodeJS debugger.

• open Memory tab, select Heap Snapshot and click Take Snapshot. You will get something like this:

Now it's time to make some requests. I'm using autocannon, but you can use any tool like ab.

autocannon -c 50 -a 1000 http://localhost:3000 

Now you can take second snapshot. It should be much bigger:

After 4K requests you will see that there is a lot of allocations in array part. For some reason there are many allocations of Controller class from react-spring. I have no idea why.

It looks like new Controller class is created for each request, but old instances are not garbage collected. (this and this lines?)

Now let's compare snapshots in this same application but without getInitialProps in _app.js - no SRR.

After 10K request you will get something like this:

• Snapshot 1 - after yarn start
• Snapshot 2 - after 10K req
• Snapshot 3 - after GC

System information

• OS: macOs / Docker 12.4.0-alpine
• Version of Next.js: 9.0.2, 9.0.3
• Version of react-spring 8.0.27, 9.0.0-beta.31
• Version of React: 16.8.6

Thank you

[RafalFilipek]

Same with 9.0.0-beta.33

[RafalFilipek]

@aleclarson I think the problem is that stop function from FrameLoop is not called.

When I add this.controllers.clear(); in start method of FrameLoop there is no leak (I know it's not a solution).

Thank you

[VindulaF]

Running into the same issue here, our Next.js app server ran into OOM issues constantly because of this, we were using react-spring: 8.0.27 and next: 7.0.2.

[RafalFilipek]

@aleclarson is there anything I can try to solve this issue? I know you are quite busy with v9 so maybe some hints :)

Thank you

[aleclarson]

Haven't had time to dig into this yet. You can use issuehunt.io to put a bounty on this issue if it's a blocker for you.

[RafalFilipek]

@aleclarson Thanks, I'll wait for new beta :) I can see you moved controllers Set inside FrameLoop. Maybe it will help.

[DeviousM]

I can confirm that this happens on Razzle & After.js combo and on the newest version of the beta (9.0.0-beta.34).

• Snapshot 1 - Right after node.js server starts
• Snapshot 2 - After first request
• Snapshot 3 - After 2600 requests.

Heap has grown substantially and after few hours on a production server it causes a crash beacuse, quite obviously, OOM.

How far is it from fixing that issue and can we expect it to be fixed in the newest beta?

[aleclarson]

@DeviousM I'm not using react-spring with SSR anywhere right now. Have you tried with the latest canary?

[DeviousM]

Yup, I've just tried the latest canary and got this error:

TypeError: Cannot read property 'ref' of null
    at useTransition (/stonly/apps/stonly-player/stonly/node_modules/@react-spring/core/index.cjs.js:2211:1)
    at /stonly/apps/stonly-player/build/webpack:/stonly-commons/components/HOC/Panel/Panel.js:104:1
    at processChild (/stonly/apps/stonly-player/build/webpack:/stonly/node_modules/react-dom/cjs/react-dom-server.node.development.js:3206:1)
    at resolve (/stonly/apps/stonly-player/build/webpack:/stonly/node_modules/react-dom/cjs/react-dom-server.node.development.js:3126:1)
    at ReactDOMServerRenderer.render (/stonly/apps/stonly-player/build/webpack:/stonly/node_modules/react-dom/cjs/react-dom-server.node.development.js:3600:1)
    at ReactDOMServerRenderer.read (/stonly/apps/stonly-player/build/webpack:/stonly/node_modules/react-dom/cjs/react-dom-server.node.development.js:3538:1)
(...rest is useless)

And the Panel.js component that's throwing that error is actually using the useTransition hook like that:

/* ...rest ommited... */
const positionMap = {
  left: 'translate3d(-100%, 0, 0)',
  right: 'translate3d(100%, 0, 0)',
};

const transition = useTransition(show, null, {
  from: { transform: positionMap[position] },
  enter: { transform: 'translate3d(0,0px,0)' },
  leave: { transform: positionMap[position] },
  config: { mass: 1, tension: 500, friction: 47 },
});
/* ...rest ommited... */

I'll provide a minimal reproduction case in a moment.

[DeviousM]

Alright, @aleclarson, I've created a repo that allows you to clone it and run the heap test locally: https://github.com/DeviousM/react-spring-ssr-issues

To reproduce the issue clone the project, then yarn to install everything and then yarn start to start it up. When you have it running, you can access the node.js's profiler tool in Chrome on localhost:7776. (you can find it at chrome://inspect)
Take a first heap snapshot after the app starts up and you'll see that it's pretty clean.

Then to simulate some load on the server download a tool like siege (that's what I'm testing with) and let it swarm the server for around 45 seconds (if you download siege then commad is siege http://localhost:3005). You can watch live how the server's heap size is growing to end up at least twice as big, mostly clogged by FrameLoop:

Let me know if you have any more questions about my example.
EDIT:// I pushed to the repo, because I kind of forgot to do that one, specific thing 😅

[aleclarson]

Yup, I've just tried the latest canary and got this error:

That's because the function signature of useTransition has changed. More info here: #809

I gave @RafalFilipek's repro a try with both 9.0.0-beta.34 and 9.0.0-canary.808.9.7e75a67.

After 10k requests, it went from 5.2MB to 44.5MB when using 9.0.0-beta.34. After installing the latest canary, it went from 5.2MB to only 8.8MB. There may or may not still be a small leak, but that's a great improvement.

You can see my fork of the repro here: https://github.com/aleclarson/repro/commits/react-spring-778

[DeviousM]

Oh, alight! Let me try it then, I'll report back with results.