Bug 201449 – Structure::storedPrototype() and storedPrototypeObject() should assert with isCompilationThread(), not !isMainThread().

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

This is a follow up to https://bugs.webkit.org/show_bug.cgi?id=201281.

[Mark Lam]

Created attachment 377959 [details]
proposed patch.

Let's try this on the EWS.

[Yusuke Suzuki]

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.

[Mark Lam]

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.

[Mark Lam]

Created attachment 377993 [details]
patch for landing.

[Mark Lam]

Thanks for the review.  Landed in r249499: <http://trac.webkit.org/r249499>.

[Radar WebKit Bug Importer]

<rdar://problem/55037141>