r/javascript • u/Warrior_monk07 • 13d ago
AskJS [AskJS] Our app didn't leak memory,it leaked memory four hundred modals later
sales kept saying the dashboard got too slow during long demos,not slow when it loaded, slow after a while, and nobody could tell me when, the best description I ever got was "maybe after lunch"...
every profiling session I ran,looked completely nice,open the app, click through the main flows, take a heap shot, flat line, ten minutes (and more) of use and no growth at all.
I closed the ticket as not reproducible twice, which I am not proud of.
Third time it came back I gave up on reproducing it in ten minutes and tried reproducing it in six hours instead, wrote a script that opened a record, opened the detail modal,closed it, moved to the next record, and did that all night, snapshot every five hundred loops, dump the heap if it dies.
by morning I had a chart going straight up and a dead browser tab at loop four thousand one hundred....yes....!
the modal added a resize listener when it opened,and the cleanup that was supposed to remove it sat behind condition that came out to be false, when you closed that modal by clicking the backdrop instead of X button,so every backdrop close left listener behind....
each one of thosee held closure,and each closure held onto that row data it rendered with,the whole record,not some small piece of it.
one leaked listener holding forty kilobytes is nothing,four hundred of them is sixteen megabytes of garbage that never goes away and plus four hundred handlers all firing every time the window resizes,which is why it showed up as slowness long before it showed up as crash,the symptom and the cause looked like two different bugs...but..:)
here is why nobody was ever going to find this by hand,you would have to open and close the same modal a few hundred times,using that one specific way of closing it,in one sitting,without refreshing,and then to notice something getting gradually worse with no clear moment where it got broked.
no tester does that,and not because they are lazy,it is just not reasonable thing to do with an afternoon or sessions end,pages get refreshed,and the moment you refresh the counter goes back to zero
the bug needed time,and time is the one thing you cannot get more of by being better at your job, everything else you can make for with skill and attention and etc,you cannot make up for eight hours of doing the same thing over and over and over again
we run that soak test every night now, it has found three more leaks since, none of them would have shown up in a session short enough for a person to sit through. this is basically what got me into QA tooling as a job, i work on autosana now and long running automated sessions are the whole point of the thing.
38
u/budd222 13d ago
"Can't find it by hand"...lol. sounds like someone who learned how to code with AI.
15
u/ThomasRedstone 13d ago
Yeah, the people doing the demos found it by hand often enough to raise three tickets for it!!!
Sitting in one of the demos would have been another option to find it.
7
u/AgenteEspecialCooper 13d ago
I have no opinion about OP's post, but I think some people could find my suggestion useful:
You can actually add a callback to an object being reclaimed by the garbage collector, and you can use that callback to throw a message in the console when an object that should have been reclaimed is stuck in memory.
Can't give you more details at this moment, but this is the class that makes the trick: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry
2
u/Landkey 12d ago
Is this useful? Am I supposed to call registry.register(target,"some value") every time I declare a new value?
1
u/hugot4eboss 12d ago
It's for debugging
2
u/Landkey 12d ago
Well, yes, but which objects and values do you register when you are hunting for an unknown villain? All of them?
1
u/hugot4eboss 12d ago
I have used it in generic setup methods that create Dom elements / classes when trying to hunt down badly behaving components. Then add same debug to specific class options to get more details
1
u/senocular 12d ago
We do something similar having scripts which run through component trees adding them to a finalization registry. We then detect when components are off-list but still in memory and flag those as potential memory leaks. In the app users can force a GC in the dev tools, and if the components are still in memory, you know there's (probably) a problem. Forcing the GC doesn't always collect everything, and there may be internal caching which gets cleaned up on its own schedule, but its a good way to find potential issues, or at least limit the components we need to look at if we've identified that there is a leak somewhere. While any object can leak, components are usually the offenders due to leftover event listeners, so what we look at first.
5
5
u/dev_and_zebra 13d ago
I remember encountering an “app gets slow over time” bug - it was a nightmare to find and a 1 s fix. In short, the app was querying thousands of records in the background killing performance. Inheritance ticket almost turned into “ I quit web dev”.
2
u/SirMcFish 13d ago
I'd have put some breakpoints on the modal open and close and probably would have seen the listener firing multiple times quite quickly.
Have had similar happen in the past and you quickly learn to spot attached events that haven't been cleaned up when you think they should.
4
1
u/Ronin-s_Spirit 12d ago
This is the type of thing you, well not you specifically (clearly), can find by hand by looking for things that get hooked up to memory when they shouldn't. Things like reapplied listeners, functions with big heavy closures, a path that generates too many objects too often etc.
1
1
u/hiddencamel 13d ago
I mean, the instant they told you it got slow over a long session you should have guessed it was a memory leak. If the primary mode of interaction is opening and closing modals, that should have told you the issue was likely with the modals.
It was a good idea to automate a long test session to replicate, but intuition alone should have had you looking at whether the modals were unmounting correctly.
2
u/phatdoof 13d ago
We had a similar deal and also suspected unmatched listeners.
We did a grep of listener pairs and didn’t find any mismatches.
We put it through AI which also didn’t find anything.
In the end we found that one dev did window.addEventListener("click", stuff); and document.removeEventListener("click", stuff);
1
u/OhKsenia 13d ago
Sounds like you learned nothing tbh. Using longer sessions and waiting until things crash/all available memory is used to debug memory leaks is the dumbest thing I've ever heard. What if you're running these tests on a machine with 128 gb of ram? Your test that takes 8 hours on your current machine may take 3 days instead.
Just opening and closing one or two modals and seeing that memory isn't being cleared should immediately tell you that you have a memory leak already.
2
u/leixiaotie 12d ago
well if the memory footprint for each listener is small (in bytes even kb range), it won't notice anything. But yeah waiting for crash is bad, because several mb increase and consistently should be noticable
101
u/name_was_taken 13d ago
Correction: It always leaked. You just didn't notice the leak until it had been used a while.
And historically, people did find this manually. I've done it myself, as a coder, investigating the same "eventually gets slow" reports. It's not fun to find, but it's part of the job.