Recoil icon indicating copy to clipboard operation
Recoil copied to clipboard

Memory leak when using atomFamily/selectorFamily

Open alanhoff opened this issue 4 years ago • 18 comments

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

Screen Shot 2020-06-19 at 12 07 12

alanhoff avatar Jun 19 '20 15:06 alanhoff

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.

trivigy avatar Nov 09 '20 04:11 trivigy

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

ltwlf avatar Jan 03 '21 01:01 ltwlf

I don't think that warning should be related.

drarmstr avatar Jan 04 '21 20:01 drarmstr

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

ianstormtaylor avatar Jan 14 '21 21:01 ianstormtaylor

@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

mondaychen avatar Feb 23 '21 18:02 mondaychen

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

trivigy avatar Feb 23 '21 20:02 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.

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

chybisov avatar Mar 31 '21 15:03 chybisov

@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.

mondaychen avatar Apr 01 '21 15:04 mondaychen

@mondaychen I think you're right. I added a comment here: https://github.com/facebookexperimental/Recoil/issues/951#issuecomment-845958082

ianstormtaylor avatar May 21 '21 13:05 ianstormtaylor

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.

mondaychen avatar May 24 '21 20:05 mondaychen

Any update on this?

KevinMusgrave avatar Sep 19 '21 04:09 KevinMusgrave

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

ofirzBM avatar Jan 05 '22 12:01 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

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

drarmstr avatar Jan 06 '22 02:01 drarmstr

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

mejackreed avatar Aug 03 '22 12:08 mejackreed

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?

luishdz1010 avatar Sep 23 '22 02:09 luishdz1010

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

drarmstr avatar Sep 23 '22 22:09 drarmstr

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

Tomas2D avatar Oct 12 '22 09:10 Tomas2D

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..

drarmstr avatar Oct 12 '22 19:10 drarmstr