facebookexperimental / Recoil

Recoil is an experimental state management library for React apps. It provides several capabilities that are difficult to achieve with React alone, while being compatible with the newest features of React.
https://recoiljs.org/
MIT License
19.59k stars 1.18k forks source link

SSR - memory leak #1864

Open Tomas2D opened 2 years ago

Tomas2D commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.