Closed Bug 1404486 Opened 2 years ago Closed 2 years ago

Permaleak of CacheIOThread in dom/base mochitests

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: mccr8, Assigned: nbp)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak)

Attachments

(1 file)

This is a permaleak, somewhere in the dom/base directory, of what looks like a networking cache:

TEST-INFO | leakcheck | tab process: leaked 1 CacheFileHandles
TEST-INFO | leakcheck | tab process: leaked 1 CacheFileIOManager
TEST-INFO | leakcheck | tab process: leaked 1 CacheIOThread
TEST-INFO | leakcheck | tab process: leaked 2 CondVar
TEST-INFO | leakcheck | tab process: leaked 2 Mutex
TEST-INFO | leakcheck | tab process: leaked 1 ThreadEventTarget
TEST-INFO | leakcheck | tab process: leaked 1 ThreadTargetSink
TEST-INFO | leakcheck | tab process: leaked 17 nsTArray_base
TEST-INFO | leakcheck | tab process: leaked 1 nsThread

This is present at least on Linux and Linux64, but it might also be on other platforms. I'll file this in the cache component, but it might be that some DOM thing is causing the leak.
Using the amazing scientific process of searching for files in the dom/base/test directory that contain the word "cache" and apparently being a skilled guesser, I have determined that dom/base/test/test_script_loader_js_cache.html seems to cause this leak. Or at least, you hit this exact leak by running that test like this:
  ./mach mochitest -f plain dom/base/test/test_script_loader_js_cache.html
Then manually exiting the browser.

Nicolas, can you investigate this? Thanks.
Component: Networking: Cache → JavaScript Engine
Flags: needinfo?(nicolas.b.pierron)
For what it is worth, there are only a couple of tests that use nsICacheTesting, and this is the only one that isn't an XPCShell test.
I have little insight in the cache implementation, I will investigate on my side on Monday.
Looking at DMD, I do not see any allocation made under the ScriptLoader which would still be alive at the end of the test.
On the other hand, I see that there is an event queue reported under the CacheFileIOManager, and a CacheIOThread which are held alive.

I do not yet know why nor how, and if these correspond to the leak which is being reported.
Valentin would you have any idea why we might keep this event queue in the CacheIOThread?
Flags: needinfo?(valentin.gosu)
The thread performs cache IO operations and uses the event queue to process them.
I ran the test with the lines containing await flushNeckoCache() commented, and the leak went away.
I'm not exactly sure why we're leaking it.
Flags: needinfo?(valentin.gosu)
(In reply to Valentin Gosu [:valentin] from comment #6)
> The thread performs cache IO operations and uses the event queue to process
> them.
> I ran the test with the lines containing await flushNeckoCache() commented,
> and the leak went away.
> I'm not exactly sure why we're leaking it.

What is this line made for, I saw it in the test cases of the alternate data type, and assumed this was mandatory for the test case to run, as we wanted to ensure that the data was accessible for later consumption.

Can we just simply remove it?
The xpcshell tests sometimes get the entry from the channel in order to check its properties. We need to flush and GC in order for the entry to go away and not be in memory all the time (as that would defeat the purpose of some of the tests)
From what I can tell, you only deal with the content the channel gives you, and the test passes without the line. You can probably remove them.
Flags: needinfo?(nicolas.b.pierron)
Attachment #8914830 - Flags: review?(valentin.gosu) → review+
Assignee: nobody → nicolas.b.pierron
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/52a87a609fcb
Do not flush necko cache to avoid keeping the CacheIOThread alive. r=valentin
Keywords: checkin-needed
Depends on: 1405873
https://hg.mozilla.org/mozilla-central/rev/52a87a609fcb
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Whiteboard: [checkin-needed-beta]
No longer depends on: 1405873
You need to log in before you can comment on or make changes to this bug.