| Summary: | Structure::storedPrototype() and storedPrototypeObject() should assert with isCompilationThread(), not !isMainThread(). | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
| Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=201281 | ||||||||
| Attachments: |
|
||||||||
|
Description
Mark Lam
2019-09-03 23:25:37 PDT
Created attachment 377959 [details]
proposed patch.
Let's try this on the EWS.
Comment on attachment 377959 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=377959&action=review r=me > Source/JavaScriptCore/runtime/StructureInlines.h:111 > + ASSERT(isCompilationThread() || object->structure() == this); Is it possible that the GC thread accesses to this functions? If so, we should allow GC thread too (I think we have an check like isCompilationThread). If not, this check is fine. > Source/JavaScriptCore/runtime/StructureInlines.h:119 > + ASSERT(isCompilationThread() || object->structure() == this); Ditto. Comment on attachment 377959 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=377959&action=review >> Source/JavaScriptCore/runtime/StructureInlines.h:111 >> + ASSERT(isCompilationThread() || object->structure() == this); > > Is it possible that the GC thread accesses to this functions? If so, we should allow GC thread too (I think we have an check like isCompilationThread). If not, this check is fine. AFAICT from grepping the code and from running tests, this is not called from GC threads. However, there is a Thread::mayBeGCThread() available. To better communicate our intent here, I'll add Thread::mayBeGCThread() to the assert clauses. Created attachment 377993 [details]
patch for landing.
Thanks for the review. Landed in r249499: <http://trac.webkit.org/r249499>. |