Riot/Web locks Chrome 80 solid for ~40s with ~12 cores of kernel

[ara4n]

Every hour or so I'm getting spectacular deadlocks on my account, which beachballs Chrome 80 on Catalina for 30-40s at a time. JS heap is ~300MB, but it looks like something may be leaking sprites? This began ~1 week ago.

Hitting the box with System Trace in Instruments (given Catalina has broken dtrace...) shows that Chrome is thrashing away doing dispatches which turn into kevents for managing a CoreImage cache?

[ara4n]

Looks like a DOM leak - i have 127,134 detached div elements hanging around.

They look like:

I took a (1.5GB) heap dump.

The unreleased facebook/react#17666 looks like it might be related, so i'm trying on react@next.

[ara4n]

The multi-minute pauses were driving me nuts, so I went on a leak mission with help from @t3chguy and @jryans. Conclusions are:

we have leaks in:

• selectionchange in BasicMessageComposer (which leaks the whole DOM tree whenever you change rooms due to race between focus/blur)
• pillifyLinks (which leaks the whole DOM tree whenever you render a pill in the timeline, via BaseAvatar's onClientSync for the avatar in the pill)
• velociraptor (for RRs) (perhaps, although it might be just for RRs in flight)
• ...and possibly everywhere we do a ratelimitedfunction or timed promise (at least until the timeout fires, although this might have been me failing to GC). I specifically noticed _updateE2eStatus in RoomTile and the LL _membersPromise in js-sdk.

This looks to be a dup of #10890.

This probably deserves a writeup similar to https://blog.discordapp.com/discord-react-memory-leak-565c89763e8 to explain the methodology and for future readers to avoid experiencing the same pain of working out how to drive dev tools to find the leak. My notes are:

• take heap snapshots using the memory profiler.
• pick something which really shouldn't ever leak (e.g. MessageComposer or RightPanel) - not detached HTMLDivElements
• pick something which isn't leaking much (leaking 250K detached divs is hard to track, but 8 messagecomposers is much easier to reason about)
• spot the granularity of what's leaking by looking at other objects which leak a similar number (e.g. 8 right panels, 8 messagecomposers, etc)
• perhaps check when the leaks happen, using the allocation timeline profile mode of the memory profilers.
• use roughly consistent repro steps (view the same series of rooms each time, in this instance)
• hit the GC button before you capture a heap (is this really needed?)
• look at the various top level retain chains for each leaked object, not just the first one (which by default will be the shortest path to the GC root, but may not be the most informative)
• look at the reactInternalFiber objects in the retain graph ("reveal in summary window") to figure out precisely which React nodes are retaining
  • for instance, turns out that the BaseAvatar retaining the composer is consistently the ones for @thelink:mozilla.org and @haxen:matrix.org and me.
• look at the __reactInternalInstance of the detached DIV in the retain graph
  • its outerText is: "14:36 TheLink: Layout problems in german i18n #12475 Layout problems in german i18n · Issue Layout problems in german i18n #12475 · vector-im/riot-web - GitHubTruncated buttons and overflowing labels."
  • this tells us what div has leaked; it's the BaseAvatar in the pill. Check where this pill comes from, and discover pillifyLinks manually messing with the DOM => leak.
• comment out any dodgy looking retainers in the code (e.g. promises which claim to be holding onto the object). these could be real leaks or could be side-effects of the actual leak (i.e. legit stuff which would get cleaned up if the real leak wasn't leaking).
  • all our RateLimitFuncs might be leaking however, if they're not cleaned up properly on unmount?
  • likewise promises for things like LL loading?
• when reading the retain chains, keep a close eye out for recurring bits of non-React code.
• by elimination, find the real leak (in this instance, BasicMessageComposer's _onSelectionChange being leaked, holding onto a this which in turn retains the whole ReactDOM, due to sloppy handling of the selectionchange event listener).

stuff which didn't work:

• moving to react@next (although it might have helped debug a bit by reducing number of spurious references between fibernodes thanks to stuff like Clean up nextEffect pointer facebook/react#16115 and Null stateNode after unmount facebook/react#17666)
• inspecting the inspector (probably easier to write a script to walk a heapdump than script chrome dev tools to do it)
• using React Dev Tools, due to Memory leak - React DOM keeps references to previous states/props/children of component facebook/react#16138 or similar meaning that the dev tools references made it even harder to read the retainer graphs.

[ara4n]

@turt2live why is riot/web locking Chrome for a minute every few hours not a fire?

[turt2live]

As far as I know it's only affecting 1 person and not widespread. It doesn't sound like a drop-everything-and-fix-it issue, but definitely one that should be p1 for when the maintainer role is out of fires to put out.

It was also missing from the board and presumed to be not a fire if it hasn't been tracked fully.

[ara4n]

This should be affecting anyone with a relatively big account. I suspect it's got worse recently either due to a Chrome upgrade (such that it now takes out the whole browser rather than killing the tab), or possibly with a recent version of React 16, causing retaining a full DOM tree to have a larger footprint. I suspect it's also the root cause of some of the symptoms described at #10890.

So yes, i'm afraid that crashing my browser every few hours counts as a drop everything and fix it, which is why I dropped everything and fixed it.

The answer to it being missing from the board is to go put it on the board rather than unhelpfully untag it.

[turt2live]

As someone with an updated Chrome and a large account, I can't say I've seen this as frequently as you've described.