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

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>