714066 - Missed FreeChunkList call in JSRuntime::onOutOfMemory and other fixes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

In the bug 702251 I changed ChunkPool::expire so it no longer frees the chunk list itself delegating that to the caller. However, I have not updated JSRuntime::onOutOfMemory to release the list of the chunks returned that the expire returns after the change leading to a chunk leak on OOM condition.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch fixes the leak and another old issue that I have found when checking for bad expire calls.

When calling startBackgroundSweep in GCCycle the code assumes that gckind != GC_LAST_CONTEXT && rt->state != JSRTS_LANDING means we should do the background sweep. However, the background sweep is setup like:

    if (gckind != GC_LAST_CONTEXT && rt->state != JSRTS_LANDING) {
        if (rt->gcHelperThread.prepareForBackgroundSweep(cx))
            cx->gcBackgroundFree = &rt->gcHelperThread;
    }

That is, the background sweep may not be available if prepareForBackgroundSweep fails on OOM. I chnaged the code so it starts the background sweep only if cx->gcBackgroundFree is not null.

Comment 2

[Igor Bukanov]

Attachment: v2

The new version fixes yet another issue - the trace JIT removal has not removed no longer needed ThreadData::waiveGCQuota. So the patch eliminates the field and related code.

Comment 3

[Igor Bukanov]

Attachment: v3

In v2 I have not eliminated everything related to waiveGCQuota. This version should purge all dead wood.

Comment 4

[Igor Bukanov]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c245807aad3a

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Backed out due to Linux make check orange:
https://tbpl.mozilla.org/php/getParsedLog.php?id=8218822&tree=Mozilla-Inbound&full=1

https://hg.mozilla.org/integration/mozilla-inbound/rev/21ff3ef9e329

Comment 6

[Igor Bukanov]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5)
> Backed out due to Linux make check orange:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=8218822&tree=Mozilla-
> Inbound&full=1
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/21ff3ef9e329

Apparently the test depend on the operation callback reporting OOM if the GC cannot shrink below gcMaxBytes. So I will land the v2 version of the patch without the latest cleanup delegating that to another bug.

Comment 7

[Igor Bukanov]

https://hg.mozilla.org/integration/mozilla-inbound/rev/91c63f6c1b13

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/91c63f6c1b13