actions menu: documentClickHandler is not removed

[pelikhan]

This is an issue that arised while using react-vega so it might have to be moved upstream.

There is a memory leak in the registeration of the actions mmenu. The document click handler is registered everytime the spec or data buffer is updated in react-vega; and it won't be garbage collected.

vega-embed/src/embed.ts

Line 434 in 641b65e

document.addEventListener('click', documentClickHandler);

Thank you for this great project!

[domoritz]

Is this vega/react-vega#383? Calling finalize should remove the handler:

vega-embed/src/embed.ts

Line 515 in 641b65e

document.removeEventListener('click', documentClickHandler);

.

[pelikhan]

I tried to trace where documentClickHandler would be removed but could not find suck code. So I think it is not related.

[domoritz]

I'm not following. I think the issue is in react vega, not embed. Do you agree?

[pelikhan]

I think this issue is not limited to react-embed. Vega registers an event callback on the document DOM element and does not provide a way to « unmount » it. Since the callback references various Vega objects in its closure, the garbage collector cannot reclaim them. My code using react-Vega happened to exacerbated this issue because I had a bug that mutated the spec JSON multiple times per second…

…

________________________________ From: Dominik Moritz ***@***.***> Sent: Wednesday, August 4, 2021 9:52:57 PM To: vega/vega-embed ***@***.***> Cc: Peli de Halleux ***@***.***>; Author ***@***.***> Subject: Re: [vega/vega-embed] actions menu: documentClickHandler is not removed (#733) I'm not following. I think the issue is in react vega, not embed. Do you agree? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fvega%2Fvega-embed%2Fissues%2F733%23issuecomment-893162583&data=04%7C01%7Cjhalleux%40microsoft.com%7C00e51e9e2e884b62d7a908d957cce4da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637637359788901540%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2fYPaLZbHioH5Optx2czeVrev8W%2F65IKoK69HmJ6PT8%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA73QKNUHMZM36KJR7CTLEDT3IKKTANCNFSM5BRUK34Q&data=04%7C01%7Cjhalleux%40microsoft.com%7C00e51e9e2e884b62d7a908d957cce4da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637637359788911537%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=raAd13k027juP9IXgcKIcKs%2BuWG47Hq7nLZ8I%2FWWVIc%3D&reserved=0>.

[domoritz]

Vega view has a finalize method that should clean that up. If it doesn't, please file an issue with a reproduction in Vega.

I'll close the issue here for now as I don't think embed has a bug.

[pelikhan]

If i am not mistakened, the callback is created by vega-embed and added to the document click handlers. I don't see how vega finalize would be able to clean up this callback.

It is declared in this scope at

vega-embed/src/embed.ts

Line 413 in 641b65e

let documentClickHandler: ((this: Document, ev: MouseEvent) => void) | undefined;

and added to document.click at

vega-embed/src/embed.ts

Line 434 in 641b65e

document.addEventListener('click', documentClickHandler);

[domoritz]

Embed has its own finalize (which calls Vega view finalize) that you need to call.