<rdar://problem/6603167> Crash in WebKit!JSC::JSGlobalObject::resetPrototype during Stress test (#3 & #7 WER crashes for Safari 4 Beta)

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Geoffrey Garen]

Patch coming.

[Geoffrey Garen]

Created attachment 28951 [details]
patch

[Oliver Hunt]

Comment on attachment 28951 [details]
patch

r=me

[Darin Adler]

Comment on attachment 28951 [details]
patch

> +    ProtectedPtr<JSDOMWindowPrototype> prototype = new JSDOMWindowPrototype(prototypeStructure.release());

Needs a comment. We so rarely use ProtectedPtr that I think it's a special event when we do use it. Something like "protect the pointer until the structure is inside a constructed xxx object".

> +    ProtectedPtr<JSWorkerContextPrototype> prototype = new (m_globalData.get()) JSWorkerContextPrototype(prototypeStructure.release());

Same.

r=me

[Geoffrey Garen]

Committed revision 41997.

[Alexey Proskuryakov]

Why do we need to use ProtectedPtr, cannot it be a plain pointer on the stack?

ProtectedPtr is evil, it uses lock, and thus increases thread contention.

[Geoffrey Garen]

There are GC solutions other than ProtectedPtr. For example, we could do something similar to what we do for JSGlobalData::exception.

I'd prefer to avoid a plain stack pointer because ensuring, in a Release build, that the pointer is visible on the stack during GC is tricky. You could try using the "volatile" keyword, but its behavior, especially with regard to automatic local variables, is notoriously surprising, and it guarantees only  that the pointer will be written to memory, but not that it will not later be overwritten before GC.

> ProtectedPtr is evil, it uses lock, and thus increases thread contention.

How evil is it? Does it show up as a problem on benchmarks?

From the comments in Collector.cpp, I see this explanation for why ProtectedPtr uses a lock: "WebCore violates this contract in Database code, which calls gcUnprotect from a secondary thread."

If the lock is truly evil, it seems easy enough to fix the one error in WebCore that necessitates its use. I recorded this design failure in <rdar://problem/5751979> almost a year ago. The only reason it wasn't fixed was that Maciej wasn't convinced that it was a user-visible problem.

[Alexey Proskuryakov]

(In reply to comment #6)
> How evil is it? Does it show up as a problem on benchmarks?

Probably not this one, but the one in ScriptValue certainly does (Oliver was going to talk to you about that recently). I'm not sure if fixing Database code is easy - at least, it wasn't easy for me when I tried.

Also, gc protection causes bug 21260, which is completely unrelated, but a sign of evilness, too.