Bug 159223 - CrashTracer beneath JSC::MarkedBlock::specializedSweep
Summary: CrashTracer beneath JSC::MarkedBlock::specializedSweep
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-28 13:27 PDT by Geoffrey Garen
Modified: 2016-06-28 14:35 PDT (History)
7 users (show)

See Also:

Attachments
Patch (3.61 KB, patch)
2016-06-28 13:32 PDT, Geoffrey Garen 		sbarati: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2016-06-28 13:27:41 PDT 
CrashTracer beneath JSC::MarkedBlock::specializedSweep
Comment 1 Geoffrey Garen 2016-06-28 13:32:13 PDT 
Created attachment 282278 [details]
Patch
Comment 2 Saam Barati 2016-06-28 14:00:48 PDT 
Comment on attachment 282278 [details]
Patch

LGTM
Comment 3 Saam Barati 2016-06-28 14:01:22 PDT 
Also, maybe you can add an assertion that doesn't allow VM entry when a GC is happening? That might help us find other bugs.
Comment 4 Saam Barati 2016-06-28 14:01:52 PDT 
Comment on attachment 282278 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282278&action=review

> Source/WebCore/ChangeLog:13
> +        In theory, other CachedResourceClients in the DOM might also trigger
> +        similar bugs, but our data only implicates the media elements, so this
> +        fix targets them.

Should we open a bug for other CachedResourceClients to vet their correctness?
Comment 5 Geoffrey Garen 2016-06-28 14:33:46 PDT 
(In reply to comment #3)
> Also, maybe you can add an assertion that doesn't allow VM entry when a GC
> is happening? That might help us find other bugs.

MarkedAllocator::tryAllocate does "ASSERT(!m_heap->isBusy())".
Comment 6 Geoffrey Garen 2016-06-28 14:34:28 PDT 
> Should we open a bug for other CachedResourceClients to vet their
> correctness?

It really wasn't clear to me from looking whether they're OK or not OK. I think we should wait for data -- and perhaps consider whether we can make re-entering JS during a destructor safe.
Comment 7 Geoffrey Garen 2016-06-28 14:35:57 PDT 
Committed r202590: <http://trac.webkit.org/changeset/202590>