Bug 186221 - throw/catch(e) abandons a Document
Summary: throw/catch(e) abandons a Document
Status: RESOLVED DUPLICATE of bug 186277
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-01 20:22 PDT by Simon Fraser (smfr)
Modified: 2018-06-04 14:45 PDT (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2018-06-01 20:22:16 PDT
If you load LayoutTests/fast/css/invalid-import-rule-insertion.html, then load another page and clear the page cache, the Document for LayoutTests/fast/css/invalid-import-rule-insertion.html is never released. It seems to be trapped in a retain cycle.
Comment 1 Radar WebKit Bug Importer 2018-06-01 20:22:33 PDT
<rdar://problem/40743441>
Comment 2 Simon Fraser (smfr) 2018-06-01 20:36:47 PDT
This is triggered by the throw and a catch that references the exception. Minimal test case: 

<script>
    try
    {
        throw('The document is abandoned');
    }
    catch(e)
    {
    }
</script>
Comment 3 Simon Fraser (smfr) 2018-06-01 20:55:59 PDT
My heap inspector tool shows a reference chain thus:

Exception (GC root—VM exceptions) -> Internal -> 
ProgramCodeBlock  ->  Internal -> 
Window  -> Variable document  -> 
HTMLDocument  “file:///Volumes/Data/Development/apple/webkit/OpenSource/LayoutTests/fast/css/invalid-import-rule-insertion.html”

m_vm->lastException() is a GC root (see slotVisitor.appendUnbarriered(m_vm->lastException()) in Heap.cpp).

Seems like we should clear this at some point.
Comment 4 Simon Fraser (smfr) 2018-06-01 20:58:29 PDT
I guess we clear lastException() next time we run script (in the VMEntryScope constructor).
Comment 5 Simon Fraser (smfr) 2018-06-01 21:26:44 PDT
Maybe GCController should call clearLastException() in some code paths.
Comment 6 Geoffrey Garen 2018-06-02 09:49:42 PDT
Two promising places to clear lastException:

(1) ~JSLock()

(2) A zero-delay timer

The purpose of lastException is to provide an out-of-band accessor to the exception thrown by the current task / micro task. So, it's OK to throw it away after the task / micro task ends.
Comment 7 Keith Miller 2018-06-04 14:45:54 PDT

*** This bug has been marked as a duplicate of bug 186277 ***