React-beautiful-dnd: Memory leak & onFatalError in DragDropContext

[faiwer]

Hello. I started to check memory leaks in our application, and found a lot of them :(
One of them relies on react-beautiful-dnd. The leak is in the DragDropContext. Let's dive in:

• We have class-component <DragDropContext/>
• This class contains: onFatalError = (error: Error) => {...}
• and onWindowError = (error: Error) => this.onFatalError(error)
• and in componentDidMount method we make a subscription: window.addEventListener('error', this.onWindowError)
• ... in componentWillUnmount - window.addEventListener('error', this.onWindowError);

What's wrong with it? These subscriptions retain in memory <DragDropContext/> instances forever. And all their links too, of course. They aren't removed from the memory because of onWindowError and onFatalError. Both of these methods are declared in the class-scope and have link to this object. So we have memory leak.

^^^ a lot of them :)

I don't know why DragDropContext needs these subscriptions, but it would be much better to remove them, or rewrite this code without memory leaks ;)

[alexreardon]

I can see the issue. It should be a simple fix. I can get to it on Monday 👌
On Fri, 17 Aug 2018 at 8:49 pm, faiwer [e-mail] wrote:

Hello. I started to check memory leaks in our application, and found a lot
of them :(
One of them relies on react-beautiful-dnd. The leak is in the
DragDropContext. Let's dive in:

• We have class-component
• This class contains: onFatalError = (error: Error) => {...}
• and onWindowError = (error: Error) => this.onFatalError(error)
• and in componentDidMount method we make a subscription: window.addEventListener('error',
  this.onWindowError)
• ... in componentWillUnmount - window.addEventListener('error',
  this.onWindowError);

What's wrong with it? These subscriptions retain in memory
instances forever. And all their links too, of course.
They aren't removed from the memory because of onWindowError and
onFatalError. Both of these methods are declared in the class-scope and
have link to this object. So we have memory leak.

[image: img_17_08_01_049a068531d]
https://user-images.githubusercontent.com/744114/44262407-c3b5a900-a23c-11e8-91fd-70cdcfa6d42a.png
^^^ a lot of them :)

[image: img_17_08_01_8efde6c043c]
https://user-images.githubusercontent.com/744114/44262447-eb0c7600-a23c-11e8-9d38-77b9f7769c6b.png

I don't know why DragDropContext needs these subscriptions, but it would
be much better to remove them, or rewrite this code without memory leaks ;)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/atlassian/react-beautiful-dnd/issues/715, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACFN7cXjgWaA4E-YsLBTNZGNS2749Slnks5uRp-igaJpZM4WBXNV
.

[faiwer]

There's also 1 more memory leak. But I haven't understood them yet. This one is more intricate. It's about this code:

  var unbind = function unbind() {
    if (!isBound) {
      return;
    }

    isBound = false;
    unbindEvents(getWindow(), pointerEvents, {
      capture: true
    });
  };

bind and unBind methods in createPostDragEventPreventer.

We again have window object and some relations with subscriptions. But.. also we have a matreshka of unbindWindowEvents :)

[alexreardon]

Let me know if you see any others
On Fri, 17 Aug 2018 at 10:26 pm, faiwer [e-mail] wrote:

There's also 1 more memory leak. But I haven't understood them yet. This
one is more intricate. It's about this code:

var unbind = function unbind() {
if (!isBound) {
return;
}

isBound = false;
unbindEvents(getWindow(), pointerEvents, {
  capture: true
});

};

bind and unBind methods in createPostDragEventPreventer.
[image: img_17_08_01_d0ed444efcd]
https://user-images.githubusercontent.com/744114/44266004-bc959780-a24a-11e8-889d-80519951fdb2.png

We again have window object and some relations with subscriptions. But..
also we have a matreshka of unbindWindowEvents :)

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/atlassian/react-beautiful-dnd/issues/715#issuecomment-413849662,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACFN7fgFsNaZKz0fF5NrmhOioRNQeyRGks5uRrZqgaJpZM4WBXNV
.

[alexreardon]

Also, what tool are you using?
On Fri, 17 Aug 2018 at 10:39 pm, Alex Reardon [e-mail] wrote:

Let me know if you see any others
On Fri, 17 Aug 2018 at 10:26 pm, faiwer [e-mail] wrote:

There's also 1 more memory leak. But I haven't understood them yet. This
one is more intricate. It's about this code:

var unbind = function unbind() {
if (!isBound) {
return;
}

isBound = false;
unbindEvents(getWindow(), pointerEvents, {
  capture: true
});

};

bind and unBind methods in createPostDragEventPreventer.
[image: img_17_08_01_d0ed444efcd]
https://user-images.githubusercontent.com/744114/44266004-bc959780-a24a-11e8-889d-80519951fdb2.png

We again have window object and some relations with subscriptions. But..
also we have a matreshka of unbindWindowEvents :)

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/atlassian/react-beautiful-dnd/issues/715#issuecomment-413849662,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACFN7fgFsNaZKz0fF5NrmhOioRNQeyRGks5uRrZqgaJpZM4WBXNV
.

[faiwer]

It's about the previous one. These unbind methods retain all their HTML & Fiber nodes.

But I can't say the certain place in the code and certain reason, that occurs this leak.

Also, what tool are you using?

I use ChromeDevTools. Their Memory tab.

• I make a snapshot of the application
• then I work with my application (click, click, click, click...)
• then click to "collect garbage" icon (trash-icon)
• then I make another snapshot
• then I compare them and search in differences what they are and why they are :)

It's a tricky deal, but it's only what I know about how to fight with memory leaks :)

[alexreardon]

I can fix the first one easily. The second one will take some more investigation