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

SSR - memory leak #1864

Open
Tomas2D opened this issue Jun 14, 2022 · 3 comments
Open

SSR - memory leak #1864

Tomas2D opened this issue Jun 14, 2022 · 3 comments

Comments

@Tomas2D
Copy link

Tomas2D commented Jun 14, 2022

Hello, firstly I would like to thank you for this fantastic tool.

Recently I have experienced a memory leak during SSR.
After I sent a response to the client, I have observed (via NodeJS debugger) that the Recoil state was not being freed.

Snímek obrazovky 2022-06-14 v 10 36 18

The image above depicts a memory snapshot after all requests have been served. The garbage collector has been explicitly called after every request (just for this test scenario). The memory usage increases with every request, leading to a crash of the whole server.

Example SSR code:

try {
    const initData = await getDataByRequest(req)

    const app = (
      <React.StrictMode>
        <Router hook={staticLocationHook(req.path)}>
          <RecoilRoot initializeState={initStore(initData)}>
            <App />
          </RecoilRoot>
        </Router>
      </React.StrictMode>
    )

    const appHtml = renderToString(sheet.collectStyles(app))
    const stylesHtml = sheet.getStyleTags()

    const helmet = Helmet.renderStatic()
    const headHtml = [
      helmet.title.toString(),
      helmet.meta.toString(),
      helmet.script.toString(),
      helmet.link.toString()
    ]
      .filter(Boolean)
      .join('\n')

    return getIndexHtml()
      .replace(/<title>(.*)<\/title>/, headHtml)
      .replace('<style id="styles-placeholder"></style>', stylesHtml)
      .replace('<div id="root"></div>', `<div id="root">${appHtml}</div>`)
      .replace(
        /window\.__PRELOADED_STATE__\s?=\s?null/,
        `window.__PRELOADED_STATE__ = ${jsesc(JSON.stringify(initData || null), {
          json: true,
          isScriptContext: true
        })}`
      )
  } finally {
    sheet.seal()
  }
@mondaychen
Copy link
Contributor

Hi! Are you using family utils? There's a known issue #366
Unfortunately it took longer than expected to fix the issue. Sorry about that!
Is it possible to manually release the app after it's rendered?

@Tomas2D
Copy link
Author

Tomas2D commented Jun 17, 2022

Yes, I use selectorFamily.

I did not found any way to release the memory, but I solved that issue by delegating the render part to the pool of fork processes, once the process exceed the allowed memory, it is killed and replaced by a new process. Simplier but not so efficient would be to create a process for each request, render and kill it afterwards, but this come with a maintenance cost.

@Tomas2D
Copy link
Author

Tomas2D commented Oct 13, 2022

I was able to get rid of memory leak within two steps:

  1. export useStoreRef function from the recoil library,
  2. create releaseStore function inside the recoil library,
diff --git a/node_modules/recoil/cjs/index.js b/node_modules/recoil/cjs/index.js
index 116fe1d..e36121c 100644
--- a/node_modules/recoil/cjs/index.js
+++ b/node_modules/recoil/cjs/index.js
@@ -9180,3 +9180,29 @@ exports.waitForAll = Recoil_index_18;
 exports.waitForAllSettled = Recoil_index_19;
 exports.waitForAny = Recoil_index_17;
 exports.waitForNone = Recoil_index_16;
+exports.useStoreRef_SSR = useStoreRef;
+
+function releaseStore(store) {
+  const state = store.getState()
+  state.knownAtoms.forEach((node) => {
+    releaseNode(store, state.currentTree, node)
+  })
+  state.knownSelectors.forEach((node) => {
+    releaseNode(store, state.currentTree, node)
+  })
+}
+
+exports.releaseStore_SSR = releaseStore

and updated my code for doing SSR,

async function renderApp({ req, state }: SSRInput['payload']): Promise<SSROutput['payload']> {
  const sheet = new ServerStyleSheet()
  let storeRef: { current: RecoilStore }

  try {
    // this block was added
    const AppWrapper = () => {
      storeRef = useStoreRef_SSR()
      return <App />
    }

    const app = (
      <Router hook={staticLocationHook(req.path)}>
        <RecoilRoot initializeState={initStore(state)}>
          <AppWrapper />
        </RecoilRoot>
      </Router>
    )

    const appHtml = renderToString(sheet.collectStyles(app))
    const stylesHtml = sheet.getStyleTags()

    const helmet = Helmet.renderStatic()
    const headHtml = [
      helmet.title.toString(),
      helmet.meta.toString(),
      helmet.script.toString(),
      helmet.link.toString()
    ]
      .filter(Boolean)
      .join('\n')

    return {
      head: headHtml,
      style: stylesHtml,
      body: appHtml
    }
  } finally {
    sheet.seal()
    // this line was added
    releaseStore_SSR(storeRef!.current)
  }
}

Would you think that there is a better approach, ideally without patching the library? @mondaychen
Thanks in advance.

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

2 participants