SSR Memory Leak with v5 release candidate 2

[brad-decker]

Environment

Default, Baseline

## System:
 - OS: macOS 10.15
 - CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
 - Memory: 3.11 GB / 32.00 GB
 - Shell: 5.6.2 - /usr/local/bin/zsh
## Binaries:
 - Node: 10.15.1 - ~/.nvm/versions/node/v10.15.1/bin/node
 - Yarn: 1.19.1 - ~/****/****/applications/site/node_modules/.bin/yarn
 - npm: 6.4.1 - ~/.nvm/versions/node/v10.15.1/bin/npm
## npmPackages:
 - babel-plugin-styled-components: ^1.10.0 => 1.10.6
 - styled-components: ^4.0.0 => 4.4.1

After upgrade to RC 2

## System:
 - OS: macOS 10.15
 - CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
 - Memory: 3.11 GB / 32.00 GB
 - Shell: 5.6.2 - /usr/local/bin/zsh
## Binaries:
 - Node: 10.15.1 - ~/.nvm/versions/node/v10.15.1/bin/node
 - Yarn: 1.19.1 - ~/****/****/applications/site/node_modules/.bin/yarn
 - npm: 6.4.1 - ~/.nvm/versions/node/v10.15.1/bin/npm
## npmPackages:
 - babel-plugin-styled-components: ^1.10.0 => 1.10.6
 - styled-components: ^5.0.0-rc.2 => 5.0.0-rc.2

Notes: Both environments use TypeScript, Next.JS and latest versions of react

Steps to Reproduce

1. Use the _document.js file found in the with-styled-components example of next.js.
2. Have styled-components 4.4.1 installed.
3. Run application using N|Solid from nodesource. Also could probably get this information by attaching chrome inspector to node.
4. Run npx loadtest -c 5 --rps 10 -k -t 90 http://localhost:8080/
5. See that memory usage has peaks and valleys, RSS and Heap memory allocation move together. External memory barely moves.
6. install release candidate 2
7. repeat the steps above.
8. See that Heap allocation remains consistent, MORE consistent than in 4.4.1, but RSS and External memory allocation seem to climb indefinitely until the application crashes under load.
9. Look at memory snapshot and see that ServerStyleSheet is massive, at around 2 gigabytes, and the server is holding onto array buffers and strings that seem to reference styles.

Memory Timeline In N|Solid on version 4.4.1

heapsnapshot

Memory Timeline in N|Solid on Version 5 RC 2

heapsnapshot

Comparison of Heap snapshots

RC 2 is on the left. The snapshots were taken as close to the same point as possible (immediately following the load test that was run with the same stats: 5 concurrent threads, keep-alive connections, 10 requests per second for 90 seconds). The only change between the two is upgraded to RC 2.

[brad-decker]

If you'd like me to provide heap snapshot dumps or screen recordings of the two tests I can, but I doubt GitHub would like me much trying to attach here. :)

[quantizor]

Some sort of profiling would definitely be helpful, thanks.

[brad-decker]

@probablyup updated the issue description to include links to heapsnapshots.

[floric]

@brad-decker I had the same issue with v5. My memory leak was caused by global styles. I replaced them by static generated CSS for now. Not sure though if this is connected with your problem.

[kitten]

Like @floric says, this could be due to some usage patterns and specific things that aren’t being used correctly.

I can take a look at the heap snapshot but I’d suspect that that won’t actually tell us much about what’s going on.

It’s probably a good idea to check whether you’re creating dynamic StyledComponents over and over again inside the render function which would lead to this sort of memory profile.

[fabb]

Like @floric says, this could be due to some usage patterns and specific things that aren’t being used correctly.

@kitten what if my global styles depend on some component prop? How would I do that safely without leaking?

[mxstbr]

@fabb that is exactly why we switched from the old injectGlobal to the new createGlobalStyle API:

const GlobalStyles = createGlobalStyle`
  background: ${props => props.bg};
`

<GlobalStyles bg="red" />

[fabb]

@mxstbr ah ok, so instead of calling createGlobalStyle inside render functions, create them on top level and parametrize them with props. I‘ll check my usages. Would be helpful though if an unsafe usage of createGlobalStyle could somehow produce a compiler/linter error or at leased a console error.

[mxstbr]

That's correct, definitely should not be re-created every render that would definitely lead to a memory leak!

@probablyup @kitten reckon we can add a warning when it's used like that?

[kitten]

@mxstbr yep, we probably can add a warning about but it’d be based on React internals

[brad-decker]

@kitten we are using global styles using the new api and not inside a render method. As far as creating many styled components in render wouldn’t that have had a similar adverse effect in v4?

[mxstbr]

As far as creating many styled components in render wouldn’t that have had a similar adverse effect in v4?

It would still be detrimental to performance, although it might not lead to a memory leak as quickly. Definitely do not create your styled components in the render cycle!!

[brad-decker]

@mxstbr if we are, which I doubt, it would be a user error rather than a chosen pattern. Any tips on hunting down where this could be happening at?

[brad-decker]

I have confirmed @kitten and @mxstbr that the only two instances of createGlobalStyles in our application are used in files to export components that are consumed in JSX, we are not calling createGlobalStyles in render methods.

[jaslakson]

Just today we bumped our prod instance up to RC2 after having passed our automated and manual regression tests in our staging environment. Approx 1hr after releasing, we saw a spike in memory usage that brought our site down. We restarted the dynos and all was well until another hour had passed - did this twice while investigating the problem. Found this thread and reverted our version change and the problem has gone away.

we use next.js and we do use createGlobalStyle - here's a snippet of how it's used in our _app.js file:

//...other imports removed for brevity
import { createGlobalStyle } from 'styled-components';
import { reset } from '../config/css';

const GlobalStyle = createGlobalStyle`${reset}`;

class MyApp extends App {
  //...code removed for brevity

  render() {
    const {
      Component,
      pageProps,
      queryParams,
      store,
      url,
    } = this.props;

    return (
      <Provider
        store={store}
      >
        <ErrorBoundary>
          <GlobalStyle />
          <Component
            {...pageProps}
            queryParams={queryParams}
            url={url}
          />
        </ErrorBoundary>
      </Provider>
    );
  }
}

The reset import is an exported template string that's basically:

import styledNormalize from 'styled-normalize';

const reset = `
  ${styledNormalize}
  html {
  ...some css here
  }
`;

export default reset;

Please let me know if there's a better/alternate way to use createGlobalStyle.

Here's a snapshot of our metrics dashboard, showing memory usage - the vertical line just after 9 and during the final spike are when we released/rolled back the RC2 change:

[quantizor]

we bumped our prod instance up to RC2

From what version?

[jaslakson]

"styled-components": "4.3.2" - to be safe, we just reverted to this version instead of the latest on 4.x

was looking through the changes for the 5.x branch and the only thing that stuck out to me as a possible memory leak is:

useEffect(() => () => globalStyle.removeStyles(instance, styleSheet), []);

could the runtime beholding onto a reference to instance or styleSheet unexpectedly - I'm certainly not an expert in the subtleties here, but is there a difference between an arrow fn and a standard function that could be at fault? I don't see anything else in the change that stood out (speaking about this one https://github.com/styled-components/styled-components/pull/2824/files). The reason this stood out is that it's a cleanup/callback setup, which sometimes can be a source of memory leaks if they reference something that should have been garbage collected.

BTW - thanks for the excellent work - really looking forward to v5.

[quantizor]

Hmm I'd hope React would drop that since during SSR unmount effects will never be called... might be a bug on their end.

[quantizor]

If you wanted to try patching the code, you could surround that effect in an "if browser" sort of check so it doesn't get run on the server

[jaslakson]

That's a good idea - I'll look into that - might be able to patch it and run some load tests on our staging servers over the course of a few hours to verify.

[fabb]

Wait, I thought useEffect won‘t be called at SSR at all, same as componentDidMount?

[quantizor]

Supposedly

[brad-decker]

It's not supposedly. it is not called on SSR.

[brad-decker]

what can I do to remove the needs more info? What additional information can I provide?

[fabb]

Could it be that the increased memory use stems from overuse of useMemo (e.g. in StyleSheetManager or ThemeProvider)?

As I read here (https://kentcdodds.com/blog/usememo-and-usecallback), useMemo needs to keep references to previous elements of both its result and its dependency array, which could cause memory growth:

As a related note, if you have dependencies then it's quite possible React is hanging on to a reference to previous functions because memoization typically means that we keep copies of old values to return in the event we get the same dependencies as given previously. The especially astute of you will notice that this means React also has to hang on to a reference to the dependencies for this equality check (which incidentally is probably happening anyway thanks to your closure, but it's something worth mentioning anyway).

[quantizor]

Oh interesting. Does useRef have the same issue?

[fabb]

No, useRef does not take a dependency array and therefore cannot do such memoizing.

[quantizor]

Definitely open to a PR against v5 switching over the use of useMemo!

[fabb]

On the other hand, v4 was using memoize-one exactly where v5 uses useMemo, so I‘m not sure this is the cause for the memory leaks of the OP.

[quantizor]

@fabb we're doing a lot more in StyleSheetManager for v5 using those hooks, it's worth some exploration.

[fabb]

Hm, the only additional thing I see memoized in StyleSheetManager is the disableCSSOMInjection dependency which should not change too often anyways I guess:

https://github.com/styled-components/styled-components/blob/canary/packages/styled-components/src/models/StyleSheetManager.js#L76 vs. https://github.com/styled-components/styled-components/blob/master/packages/styled-components/src/models/StyleSheetManager.js#L30

[fabb]

Ah, I looked at the canary branch instead of the v5 branch 🙈

[fabb]

Also very interesting: https://github.com/facebook/react/issues/15278

[fabb]

The useMemo suspect probably is a red herring, I wrote a unit test to verify if previous values are kept and could not verify that: Codesandbox

[chaffeqa]

FYI we've been using 5.0.0-rc.3 in prod and we definitely had memory leak issues w SSR

[quantizor]

If you can do some profiling to help figure it out, that would be great. We don’t have any obvious leads at the moment.

On Mon, Jan 6, 2020 at 1:38 PM Quinn Chaffee notifications@github.com wrote:

FYI we've been using 5.0.0-rc.3 in prod and we definitely had memory leak issues w SSR

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/styled-components/styled-components/issues/2913?email_source=notifications&email_token=AAELFVQDYEQBSPW7YCKRQW3Q4N3A5A5CNFSM4JVAJKXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIGLB7Y#issuecomment-571257087, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAELFVQEGUTRCPEWFBZMSZDQ4N3A5ANCNFSM4JVAJKXA .

[chaffeqa]

unfort we werent able to get great info... looks like the sourcemaps werent working.

Here's a sentry we were getting: https://sentry.io/share/issue/cb04aca756984f7a87c9ed8391083eb7/

in case you cant see that:

RangeError: Array buffer allocation failed
  ?, in new ArrayBuffer
  ?, in new Uint32Array
  File "app:///styled-components.cjs.js", line 417, col 25, in DefaultGroupedTag.insertRules
    this.groupSizes = new Uint32Array(newSize);
  File "app:///styled-components.cjs.js", line 654, col 19, in StyleSheet.insertRules
    this.getTag().insertRules(getGroupForId(id), rules);
  File "app:///styled-components.cjs.js", line 1644, col 16, in GlobalStyle.createStyles
    styleSheet.insertRules(id, id, css);
  File "app:///styled-components.cjs.js", line 1655, col 10, in GlobalStyle.renderStyles
    this.createStyles(instance, executionContext, styleSheet);
  File "app:///styled-components.cjs.js", line 1684, col 19, in GlobalStyleComponent
    globalStyle.renderStyles(instance, STATIC_EXECUTION_CONTEXT, styleSheet);
  File "app:///react-dom-server.node.production.min.js", line 36, col 498, in d
    '{snip} vedStateFromProps){var w=h.getDerivedStateFromProps.call(null,d.props,e.state);null!=w&&(e.state=k({},e.state,w))}}else if(O={},e=h(d.props,
  File "app:///react-dom-server.node.production.min.js", line 39, col 16, in Za
    typeof h)break;d(f,h)}return{child:a,context:b}}
  File "app:///react-dom-server.node.production.min.js", line 44, col 476, in a.b.render
    '{snip} turn N(f);if(this.previousWasTextNode)return"\x3c!-- --\x3e"+N(f);this.previousWasTextNode=!0;return N(f)}b=Za(a,b,this.threadID);a=b.child;
  File "app:///react-dom-server.node.production.min.js", line 44, col 18, in a.b.read
    w="";try{w+=this.render(l,e.context,e.domNamespace)}catch(t){if(null!=t&&"function"===typeof t.then)throw Error(r(294));throw t;}finally{}h. {snip}
  File "app:///react-dom-server.node.production.min.js", line 54, col 462, in renderToStaticMarkup
    '{snip} rToStaticMarkup:function(a){a=new $a(a,!0);try{return a.read(Infinity)}finally{a.destroy()}},renderToNodeStream:function(a){return new bb(a,
  File "app:///react-ssr.cjs.js", line 38, col 16, in process
    var html = renderFunction(_react.default.createElement(ApolloContext.Provider, {
  File "internal/process/task_queues.js", line 93, col 5, in processTicksAndRejections
  File "internal/process/task_queues.js", line 62, col 3, in runNextTicks
  File "internal/timers.js", line 412, col 9, in processImmediate

also:

RangeError: Source is too large
  ?, in Uint32Array.set
  File "app:///styled-components.cjs.js", line 418, col 23, in DefaultGroupedTag.insertRules
    this.groupSizes.set(oldBuffer);
  File "app:///styled-components.cjs.js", line 654, col 19, in StyleSheet.insertRules
    this.getTag().insertRules(getGroupForId(id), rules);
  File "app:///styled-components.cjs.js", line 1644, col 16, in GlobalStyle.createStyles
    styleSheet.insertRules(id, id, css);
  File "app:///styled-components.cjs.js", line 1655, col 10, in GlobalStyle.renderStyles
    this.createStyles(instance, executionContext, styleSheet);
  File "app:///styled-components.cjs.js", line 1684, col 19, in GlobalStyleComponent
    globalStyle.renderStyles(instance, STATIC_EXECUTION_CONTEXT, styleSheet);
  File "app:///react-dom-server.node.production.min.js", line 36, col 498, in d
    '{snip} vedStateFromProps){var w=h.getDerivedStateFromProps.call(null,d.props,e.state);null!=w&&(e.state=k({},e.state,w))}}else if(O={},e=h(d.props,
  File "app:///react-dom-server.node.production.min.js", line 39, col 16, in Za
    typeof h)break;d(f,h)}return{child:a,context:b}}
  File "app:///react-dom-server.node.production.min.js", line 44, col 476, in a.b.render
    '{snip} turn N(f);if(this.previousWasTextNode)return"\x3c!-- --\x3e"+N(f);this.previousWasTextNode=!0;return N(f)}b=Za(a,b,this.threadID);a=b.child;
  File "app:///react-dom-server.node.production.min.js", line 44, col 18, in a.b.read
    w="";try{w+=this.render(l,e.context,e.domNamespace)}catch(t){if(null!=t&&"function"===typeof t.then)throw Error(r(294));throw t;}finally{}h. {snip}
  File "app:///react-dom-server.node.production.min.js", line 54, col 462, in renderToStaticMarkup
    '{snip} rToStaticMarkup:function(a){a=new $a(a,!0);try{return a.read(Infinity)}finally{a.destroy()}},renderToNodeStream:function(a){return new bb(a,
  File "app:///react-ssr.cjs.js", line 38, col 16, in process
    var html = renderFunction(_react.default.createElement(ApolloContext.Provider, {
  File "./src/server/middleware/website.tsx", line 106, col 36, in data
    {fn(After)({ routes, data })}
  File "./src/client/Base/Document.tsx", line 30, col 17, in async Function.getInitialProps
    assets, data, ...page, styleTags,
  File "./src/server/middleware/website.tsx", line 36, col 81, in async dt
    const renderServerSide = async (req: express.Request, res: express.Response) => {
  File "async /app/build/server.js", line 293, col 231960, in null.<anonymous>

lemme know if that helps

[chaffeqa]

Our SSR looks like:

export default class Document extends React.Component<Props, {}> {
  public static async getInitialProps({ assets, data, renderPage }: any) {
    const sheet = new ServerStyleSheet();
    try {
      const page = await renderPage((App: any) => (props: Props) => sheet.collectStyles(<App {...props} />));
      const styleTags = sheet.getStyleElement();
      return {
        assets, data, ...page, styleTags,
      };
    } finally {
      sheet.seal();
    }
  }

  public render() {
    const {
      css,
      styleTags,
    } = this.props as Props;
    return (
      <html>
        <head>
          {css && <style id="jss-server-side">{css}</style> || null}
          {styleTags}
        </head>
        <body>
        </body>
      </html>
    );
  }
}

[kitten]

This is most likely to happen when you create StyledComponents dynamically. There’s an explanation of this here: https://github.com/styled-components/styled-components/issues/2753#issuecomment-533073888

I suspect this is the same issue, but it’s hard to know for sure. We haven’t put in any warnings for this case yet, since it’s hard to pin down a heuristic for when this usage pattern comes up.

If you’re creating a lot of unique components inside your render method (calling styled repeatedly/dynamically in render or otherwise) you’ll eventually generate enough IDs to cause an overflow. This has also been an issue in v4 but wouldn’t lead to a catastrophic error, but instead just be very slow.

It may also be caused by calling withComponent dynamically or other ways of recreating a lot of StyledComponent instances.

Hope this is the issue; maybe you could monkey patch styled to see whether this is what’s going on?

[chaffeqa]

if it helps, we have been explicit to not create dynamic styled components in our application for that reason. We rarely even pass in props and rely on class names to toggle behavior.

With that information I'd say it has to be something else?

[kitten]

@chaffeqa if I create a branch that explicitly errors on creation when the uint max limit is reached, do you think you can pin down where it may be happening?

Since we’re seeing a growing amount of memory and eventual errors when that maximum length of our array buffer is reached, it’s probably safe to assume something may be allocating these IDs

So maybe with an early error we’ll be able to investigate the stack trace?

[quantizor]

I think I know what it is... figuring out how to address it.

https://github.com/styled-components/styled-components/blob/dff9510b0b0c7da1dfd4f5794906ca7c63df4c28/packages/styled-components/src/constructors/createGlobalStyle.js#L22

This is fine client side but on the server I think this is incrementing unbounded, which eventually runs out of memory.

[quantizor]

Can y'all try out styled-components@5.0.0-cgsmem and let me know how it goes?

[brad-decker]

i'm going to give this a shot tomorrow and will report back asap.

[sammysaglam]

With rc2 & rc3 I was getting the same error as https://github.com/styled-components/styled-components/issues/2913#issuecomment-571259068. So, I tried the @5.0.0-cgsmem and seems to have stabilized (but it usually takes 12-24hrs for the crash to occur, so will post here if i see it again).

however @5.0.0-cgsmem seems to have swapping out the theme on <ThemeProvider theme={theme}> broken => theme changes, but some styles seem to not. not sure if related or if is a known issue. let me know if i can provide a running example that reproduces issue.

[jaslakson]

I will see what I can do to get a release going with cgsmem - I will have to test it on our staging servers and then push it to prod for a "real" test. Will report back with results when possible.

[quantizor]

There were no changes to theme propagation in that build. Might want to use yarn resolutions to ensure the same exact version of the library is used everywhere, etc

On Wed, Jan 8, 2020 at 10:00 AM Jason Aslakson notifications@github.com wrote:

I will see what I can do to get a release going with cgsmem - I will have to test it on our staging servers and then push it to prod for a "real" test. Will report back with results when possible.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/styled-components/styled-components/issues/2913?email_source=notifications&email_token=AAELFVQQSJKSM2GUO6SKNJLQ4XTBHA5CNFSM4JVAJKXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIM226A#issuecomment-572108152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAELFVVJEF2V23ZPGTNILBDQ4XTBHANCNFSM4JVAJKXA .

[jaslakson]

Was able to get the vsmem build into production this afternoon and so far, so good. We had previously seen errors after about an hour - release has been out for just about 2 hours and memory usage is completely flat. Will continue to monitor and post here if there is any change. For now it looks fixed. Thank you so much!

[UPDATE] - All weekend in production on this release and no memory leak issues.

[quantizor]

Thanks @jaslakson! @chaffeqa @brad-decker have you folks been able to test out this fix build yet?

[brad-decker]

Building the application with the cgsmem now. I'll report back soon, sorry for the delay @probablyup