Memory leak when using atomFamily/selectorFamily

[alanhoff]

Hello there,

The resources created by atomFamily and selectorFamily are not being properly freed when the specific key changes or if the component using the related RecoilState<T> unmounts.

Since I believe that memory management isn't properly implemented yet, both utils should be marked as _UNSAFE.

https://codesandbox.io/s/cranky-booth-penix?file=/src/App.js

[trivigy]

Could this be related? I am getting this warning and cannot seem to figure out a way to get rid of it.

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

[ltwlf]

I have the same issue like @trivigy in a unit test with react testing library. Can't get rid of the warning.

[drarmstr]

I don't think that warning should be related.

[ianstormtaylor]

I'm running into the same error as @trivigy and @ltwlf and can't figure out what's causing it.

[mondaychen]

@trivigy did you unmount RecoilRoot or just component using the selector? If the whole root then it might related to what I found in #897

[trivigy]

@mondaychen Still experience the same issue but I just squint my eye to not see so it wouldn't bother me. 😆

[chybisov]

Could this be related? I am getting this warning and cannot seem to figure out a way to get rid of it.

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

I've started to see errors like that more often with version 0.2.0. Though I don't use async selectors.

[mondaychen]

@trivigy @ltwlf @chybisov @ianstormtaylor I don't think the warning is related to the memory issue related to family utils. Let's start a new thread to discuss that. It will be really helpful if anyone can provide a way to reproduce the issue you are describing and create a new issue with that.

[ianstormtaylor]

@mondaychen I think you're right. I added a comment here: #951 (comment)

[mondaychen]

Just FYI we are working on the memory issue with use of family utils, and should have something to announce about it in the near future.

[KevinMusgrave]

Any update on this?

[ofirzBM]

Hi All,

Can we use 'most-recent' instead of 'keep-all' here in atom Family?
https://github.com/facebookexperimental/Recoil/blob/main/packages/recoil/recoil_values/Recoil_atomFamily.js#L90

[drarmstr]

Hi All,

Can we use 'most-recent' instead of 'keep-all' here in atom Family? https://github.com/facebookexperimental/Recoil/blob/main/packages/recoil/recoil_values/Recoil_atomFamily.js#L90

That would mean only the most recent atom used in the family would be retained.

[mejackreed]

Hi @mondaychen . Does the recent release (0.7.4) address any of this, or are those leaks elsewhere?

[luishdz1010]

My current solution is to create all atom families in a React Context and useMemo them. Then components can do:

const { myAtomFamily } = useAppAtoms()
const value = useRecoilValue(myAtomFamily(key))

This way the created atoms are scoped to the lifetime of the Context.Provider and I don't need to fiddle the the cache strategy.

This approach does releases the atom objects themselves when the app unmounts
(or a server render finishes), but they are still referenced by https://github.com/facebookexperimental/Recoil/blob/main/packages/recoil/core/Recoil_Node.js#L92, and also triggers warnings for duplicated Atoms after the second render.

Is there any doc outlining how memory management works in recoil? Is this strategy flawed?

[drarmstr]

That solution only controls the lifetime of the atom objects or the atom family closure which contains the dictionary of cached atom objects. While removing that would remove some minimal memory, the actual registered atom accounting structures, selectors, and selector caches would still be held in the runtime. If you would like to explore or finish memory management in Recoil use the recoil_memory_managament_2020 feature flag in Recoil_gkx.js

[Tomas2D]

@drarmstr can you be more specific on how we can achieve it?

[drarmstr]

You can add recoil_memory_management_2020 to gks in Recoil_gkx.js and then explore with retainedBy_UNSTABLE atom option, useRetain(), etc.

Would be interesting if someone wanted to do a PR for mapping GK flags here with the new RecoilEnv.js mechanism to more easily dynamically adjust features without rebuilding..

[AkifumiSato]

The RECOIL_GKS_ENABLED has already been taken in and the recoil_memory_management_2020 appears to be set by default.
What else should be done to eliminate memory leaks?

[taylorcode]

retainedBy_UNSTABLE does not appear to work, at least not in version 0.7.6. #2173

The linked PR (#2143) does not fix this issue

recoil_memory_management_2020 is enabled by default in this version.

If the retainedBy: 'components' option worked it would be amazing...Automatic cleanup of atoms when no components depend on them directly or transitively through other selectors, really great idea.

[Tomas2D]

Maybe guys you could try something I did #1864 (comment)

[tien]

Building on @Tomas2D finding, this is my solution for now until retainedBy is fully functional.
You'll need to patch recoil to exports releaseNode & useStoreRef

import { useCallback, useEffect } from 'react'
import { releaseNode, useGetRecoilValueInfo_UNSTABLE, useRecoilCallback, useStoreRef } from 'recoil'

type RECOIL_GARBAGE_COLLECTOR_UNSTABLE_Props = {
  shouldCheckForGarbageCollection: (key: string) => boolean
  interval: number
}

/**
 * TODO: remove after `retainedBy` support is implemented on recoil
 */
export const RECOIL_GARBAGE_COLLECTOR_UNSTABLE = (props: RECOIL_GARBAGE_COLLECTOR_UNSTABLE_Props) => {
  const storeRef = useStoreRef()
  const getRecoilValueInfo = useGetRecoilValueInfo_UNSTABLE()

  const releaseUnusedNodes = useRecoilCallback(
    ({ snapshot }) =>
      () => {
        if (storeRef.current === null) {
          return
        }

        const releaseSnapshot = snapshot.retain()
        const state = storeRef.current.getState()

        for (const node of snapshot.getNodes_UNSTABLE()) {
          if (!state.knownAtoms.has(node.key) && !state.knownSelectors.has(node.key)) {
            continue
          }

          if (props.shouldCheckForGarbageCollection(node.key)) {
            const firstNodeSubscriber = snapshot.getInfo_UNSTABLE(node).subscribers.nodes[Symbol.iterator]().next()

            // Can't use `snapshot.getInfo_UNSTABLE` here
            // https://github.com/facebookexperimental/Recoil/issues/1684
            const firstComponentSubscriber = getRecoilValueInfo(node).subscribers.components[Symbol.iterator]().next()

            const hasNoSubscriber =
              firstNodeSubscriber.done &&
              firstNodeSubscriber.value === undefined &&
              firstComponentSubscriber.done &&
              firstComponentSubscriber.value === undefined

            if (hasNoSubscriber) {
              releaseNode(storeRef.current, state.currentTree, node.key)
            }
          }
        }

        releaseSnapshot()
      },
    [getRecoilValueInfo, props, storeRef]
  )

  useEffect(() => {
    const interval = setInterval(releaseUnusedNodes, props.interval)

    return () => clearInterval(interval)
  }, [props.interval, releaseUnusedNodes])

  return null
}

Which can be used like so

<RECOIL_GARBAGE_COLLECTOR_UNSTABLE
  interval={5_000}
  shouldCheckForGarbageCollection={useCallback(
    node => node.startsWith('MyState') || node.startsWith('SomeOtherState'),
    []
  )}
/>

This will basically check for & release all unused nodes every [x configurable interval].