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

Memory leak on React > 16.2.5 #14732

Closed
mgreer opened this issue Jan 30, 2019 · 22 comments
Closed

Memory leak on React > 16.2.5 #14732

mgreer opened this issue Jan 30, 2019 · 22 comments
Labels
Resolution: Stale Automatically closed due to inactivity

Comments

@mgreer
Copy link

mgreer commented Jan 30, 2019

Do you want to request a feature or report a bug?
Possible bug, or unexpected change in internal behavior

What is the current behavior?
We are seeing a substantial memory leak in our codebase in versions of React > 16.5.2. Bisecting the issue, it seemed to appear at commit 7bee9fb.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

Our apologies, we are unable to fully isolate the issue to a minimal demo, and suspect it is not a bug but a behavior change which our code relied on. The leak seems to be in the messages area, where many components are created and released quickly.

What is the expected behavior?
Old fibers should be garbage collected, but cannot be. Components are retained in memory.
We have run the affected code area in StrictMode, which allowed us to identify some event leakage but even when this was addressed (by moving listeners to componentDidMount from the constructor) the memory leak continues. There are some findDomNode usages remaining, but even if we move those it does not fill the leak.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
16.5.3+

We are seeking suggestions on what internal could be causing retention of components and nodes by our code, and better ways to isolate the issue.

Thank you!
-Mike

@mgreer mgreer changed the title Unable to solve for memory leak > on React 16.2.5 Memory leak on React > 16.2.5 Jan 30, 2019
@Dergash
Copy link

Dergash commented Jan 31, 2019

We've encountered similiar issue on 16.6.3.
Allocation instrumentation on timeline shows that our application allocates a lot of FiberNode objects:
allocation
And timeline shows that they are never collected, heap grows up without GC firing when we expecting it:
react-16 6 3
At the same time, timeline on 16.8.0-alpha.1 (16.7.0 shows the same) GC seems to fire correctly:
react-16 8 0-alpha 1

May be it was already fixed by this?
0e9cb3f It's tagged 16.7.0

@mgreer
Copy link
Author

mgreer commented Jan 31, 2019

Sadly no, that had been our hope but leak persists.

@proluk
Copy link

proluk commented Jan 31, 2019

We had similar problem but we fixed that by upgrading react-dom and react lib to 16.7

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

The only relevant fix I'm aware of is #14276 which got into 16.7.0. We'd need a reproducing case to help more.

Remember to test production version, and also that we retain a reference to deleted children for one more update (#12420 (comment)).

@ejallday
Copy link

@mgreer take a look at #14284 Any of your components have <input type="password"> near the problem?

@Pitasi
Copy link

Pitasi commented Apr 30, 2019

Updating from React 16.6 to 16.8 drastically improved our memory usage, but it's still there, slow and inexorable.

Currently I have installed react and react-dom 16.8.6.

More info: I'm using Electron, BlueprintJS and Redux, I have a polling component that updates itself making some http requests and dispatching a Redux action.

From devtools I can see a lot of FiberNode and they are never deleted, they just keep growing.

My naive implementation of the component it's something like this:

class MyComponent extends Component {
  componentDidMount = () => {
    this.interval = setInterval(this.tick, 1000);
  };

  componentWillUnmount = () => {
    clearInterval(this.interval);
  };

  tick = async () => {
    if (this.updating) {
      return;
    }

    this.updating = true;
    try {
      // dispatch async (thunk middleware) redux action,
      // this will trigger an API and maybe a re-render this component
      await this.props.doUpdate();
    } catch (e) {
      /* handle error */
    }
    this.updating = false;
  };

  render() {
    <span>{this.props.myState}</span>
  }
}

if I can help providing any information, just let me know. Thanks!

@raRaRa
Copy link

raRaRa commented May 8, 2019

I have very similar memory leak with video and audio elements, perhaps related in some way? Do you find a lot of detached nodes if you create a heap snapshot under memory and search for detached in the class filter? You can check out my issue here, which is very easy to reproduce with a simple demo: #15583

@hamham91
Copy link

We have a somewhat similar memory leak (although we're experiencing it on all React 16.x versions) where there are a lot of detached nodes that never get garbage collected. The issue goes away if we downgrade to React 15.x.

You can see the documentation in this issue (along with a reproducible example): #15637

@magmaliang
Copy link

I've got the same problem. Any one solved this problem except demotion?

@tlenex
Copy link

tlenex commented May 24, 2019

There is an open PR #15157.

@qianlongdoit
Copy link

The only relevant fix I'm aware of is #14276 which got into 16.7.0. We'd need a reproducing case to help more.

Remember to test production version, and also that we retain a reference to deleted children for one more update (#12420 (comment)).

This case maybe helpful.
Potential memory leak when navigating between pages
image

@jhgg
Copy link

jhgg commented Jun 5, 2019

Here is a repro: https://codesandbox.io/s/compassionate-fermat-3cq5b

Simply uncomment line 7 and the page will continue to grow in heap until it OOMs.

@i8ramin
Copy link

i8ramin commented Jun 23, 2019

Not sure if related somehow, but I've been seeing this message a lot and the app takes a long time to hot reload during local development:

error [BABEL] Note: The code generator has deoptimised the styling of

Navigating to certain pages just hangs the browser and I have to kill the task. This seems to have started recently when we updated some packages.

@278kunal
Copy link

Any update on this ?

@gaearon
Copy link
Collaborator

gaearon commented Jul 26, 2019

#16115 was recently merged and will be out in 16.9.0. It might improve things.

@SecondFlight
Copy link

SecondFlight commented Aug 6, 2019

Upgrading to 16.9.0 completely fixes the problem for me. 🎉

@abirmingham
Copy link

Oddly, upgrading to 16.9.0-rc.0 decreases my memory leak by an order of magnitude (25MB -> 3MB per page transition), but only when NODE_ENV=development. When I use NODE=ENV=production, the leak reappears.

@gaearon
Copy link
Collaborator

gaearon commented Aug 10, 2019

What minifier are you using? We’ve seen perf issues caused by a buggy minifier version.

@abirmingham
Copy link

abirmingham commented Aug 13, 2019

Very interesting, @gaearon, thanks for the tip. I'm using uglifyjs-webpack-plugin@1.2.4 and uglify-js@3.4.10. Disabling minification via module.exports.optimization.minimize: false in webpack.config.js cuts the leak from 10MB per tree remount to 5MB. I will continue investigation.

image

EDIT: I was able to resolve a couple memory leaks in my own application. Unfortunately this didn't move the needle. However, applying #15157 cut the leak from 5MB to <1MB. Hopefully that MR will go in soon :)

EDIT: Issue is ongoing. Here are a collection of heap snapshots with 10 tree remounts in between each snapshot. This is without #15157 applied. It appears that react is the culprit, but perhaps devtools is misleading here, and user-land memory leaks will appear to originate in react? Any advice greatly appreciated! :)
image

@golden-mountain
Copy link

golden-mountain commented Dec 23, 2019

@gaearon and all,
I can easily reproduce it using react-dnd, here are my steps when I was working on following nested IDE projects, from below image we can see a lot of framed boxes, these boxes can be dragged and dropped.

ide

if I use both drag and drop, will raise memory leak
my code:

  1. setup drop
    `
    const [{ isOverCurrent }, drop] = useDrop({
    accept: [DND_IDE_NODE_TYPE, DND_IDE_SCHEMA_TYPE, DND_IDE_DATASOURCE_TYPE],
    canDrop: (item: DragItem, monitor: DropTargetMonitor) => {
    if (!refDrop.current) {
    return false;
    }
    const draggingNode = item.uinode;
    const hoverNode = uinode;

     if (draggingNode) {
       let cachedActiveTab = JSON.parse(
         localStorage.cachedActiveTab || "{}"
       );
       if (!_.isEmpty(cachedActiveTab)) {
         if (
           draggingNode.schema &&
           draggingNode.schema.$template &&
           draggingNode.schema.$template === cachedActiveTab.tabName
         ) {
           return false;
         }
       }
    
       // Don't replace items with themselves
       if (
         draggingNode === hoverNode ||
         !monitor.isOver({ shallow: true })
       ) {
         return false;
       }
     }
     if (item.type === DND_IDE_NODE_TYPE) {
       if (!dndNodeManager.canDrop(draggingNode, hoverNode)) return false;
       const hoverBoundingRect = refDrop.current!.getBoundingClientRect();
    
       // Determine mouse position
       const clientOffset = monitor.getClientOffset();
    
       // detect update region style
       const regionName = regionDetector.detectCurrentRegion(
         clientOffset as XYCoord,
         hoverBoundingRect
       );
    
       // if center, and collapsed, don't go on insert
       if (regionName === "center" && isCollapsed) return false;
    
       // judge center
       // it's not a container
       if (
         regionName === "center" &&
         !dndNodeManager.canDropInCenter(hoverNode)
       ) {
         return false;
       }
     }
     return true;
    

    },
    hover: async (item: DragItem, monitor: DropTargetMonitor) => {
    if (!monitor.canDrop()) return;

     let regionStyles;
     switch (item.type) {
       case DND_IDE_NODE_TYPE:
         // Determine rectangle on screen
         const hoverBoundingRect = refDrop.current!.getBoundingClientRect();
    
         // Determine mouse position
         const clientOffset = monitor.getClientOffset();
    
         // detect update region style
         const regionName = regionDetector.detectCurrentRegion(
           clientOffset as XYCoord,
           hoverBoundingRect
         );
    
         if (regionName) {
           if (regionName === "center") {
             regionStyles = {
               border: "3px solid #f00"
             };
           } else {
             const mapBorder = {
               up: "top",
               down: "bottom",
               left: "left",
               right: "right"
             };
             const borderName = `border${_.upperFirst(
               mapBorder[regionName]
             )}`;
             regionStyles = {
               [borderName]: "3px solid #f00"
             };
           }
           setOverClass("node");
           setBorder(regionStyles);
         }
         break;
       case DND_IDE_SCHEMA_TYPE:
         regionStyles = {
           border: "3px solid #80c35f",
           backgroundColor: "#def9d1"
         };
         setOverClass("schema");
         setBorder(regionStyles);
         break;
       case DND_IDE_DATASOURCE_TYPE:
         regionStyles = {
           border: "3px solid #80c35f",
           backgroundColor: "#40a9ff"
         };
         setOverClass("datasource");
         setBorder(regionStyles);
         break;
     }
    

    },
    drop: async (item: DragItem, monitor: DropTargetMonitor) => {
    if (!monitor.canDrop()) return;
    const draggingNode = item.uinode;
    const targetNode = uinode;

     switch (item.type) {
       case DND_IDE_NODE_TYPE:
         // Determine rectangle on screen
         const hoverBoundingRect = refDrop.current!.getBoundingClientRect();
    
         // Determine mouse position
         const clientOffset = monitor.getClientOffset();
    
         // detect update region style
         const regionName = regionDetector.detectCurrentRegion(
           clientOffset as XYCoord,
           hoverBoundingRect
         );
    
         const insertMethodName = `insert${_.upperFirst(regionName)}`;
         if (dndNodeManager[insertMethodName]) {
           dndNodeManager[insertMethodName](draggingNode, targetNode);
         }
         break;
       case DND_IDE_SCHEMA_TYPE:
       case DND_IDE_DATASOURCE_TYPE:
         if (_.isObject(item.schema)) {
           dndNodeManager.useSchema(targetNode, item.schema);
         }
         break;
     }
     setDropNode(draggingNode);
     updateSchema(targetNode.schema);
    

    },

    collect: monitor => ({
    isOverCurrent: monitor.isOver({ shallow: true })
    })
    });

`

  1. setup drag using useDrag
    const [, drag] = useDrag({ item: { type: DND_IDE_NODE_TYPE, uinode } });

  2. use drag and drop at same element, found the memory increasing much
    drag(drop(refDrop));
    drag-drop

  3. use drop only, no memory leak

  4. use drag only, memory increasing much

@stale
Copy link

stale bot commented Apr 12, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Apr 12, 2020
@stale
Copy link

stale bot commented Apr 19, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@stale stale bot closed this as completed Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Stale Automatically closed due to inactivity
Projects
None yet
Development

No branches or pull requests