Currently, JSMainThreadExecState::call() does not clear any pending exceptions in the VM before returning. On returning, the JSMainThreadExecState destructor may re-enter the VM to notify MutationObservers. This can result in a crash because the VM expects exceptions to be cleared at entry.
<rdar://problem/16525609>
Created attachment 229145 [details] the patch
Comment on attachment 229145 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=229145&action=review r=me > Source/JavaScriptCore/bindings/ScriptFunctionCall.cpp:145 > + JSValue returnedException; > if (m_callHandler) > - result = m_callHandler(m_exec, function, callType, callData, thisObject, m_arguments); > + result = m_callHandler(m_exec, function, callType, callData, thisObject, m_arguments, &returnedException); > else > - result = JSC::call(m_exec, function, callType, callData, thisObject, m_arguments); > + result = JSC::call(m_exec, function, callType, callData, thisObject, m_arguments, &returnedException); > > - if (m_exec->hadException()) { > + if (returnedException) { > hadException = true; > return Deprecated::ScriptValue(); > } Previously, this function did not clear the VM's exception. Now, it does. Was that behavior change intentional? If so, you should explain your intent in the ChangeLog, and add a test for this case to your test case. > Source/JavaScriptCore/bindings/ScriptFunctionCall.h:74 > + typedef JSC::JSValue (*ScriptFunctionCallHandler)(JSC::ExecState* exec, JSC::JSValue functionObject, JSC::CallType callType, const JSC::CallData& callData, JSC::JSValue thisValue, const JSC::ArgList& args, JSC::JSValue* returnedException); Let's just call it "exception". > Source/JavaScriptCore/inspector/InspectorEnvironment.h:37 > +typedef JSC::JSValue (*InspectorFunctionCallHandler)(JSC::ExecState* exec, JSC::JSValue functionObject, JSC::CallType callType, const JSC::CallData& callData, JSC::JSValue thisValue, const JSC::ArgList& args, JSC::JSValue* exception); Ditto. > Source/JavaScriptCore/runtime/CallData.cpp:42 > +JSValue call(ExecState* exec, JSValue functionObject, CallType callType, const CallData& callData, JSValue thisValue, const ArgList& args, JSValue* returnedException) Ditto. > Source/WebCore/ChangeLog:64 > + * bindings/js/JSNodeFilterCondition.cpp: > + (WebCore::JSNodeFilterCondition::acceptNode): Needs a comment explaining why FilterCondition wants to pass through its exception. > Source/WebCore/bindings/js/JSMainThreadExecState.cpp:49 > +JSC::JSValue functionCallHandlerFromAnyThread(JSC::ExecState* exec, JSC::JSValue functionObject, JSC::CallType callType, const JSC::CallData& callData, JSC::JSValue thisValue, const JSC::ArgList& args, JSC::JSValue* returnedException) Ditto. > Source/WebCore/bindings/js/JSMainThreadExecState.h:69 > + ASSERT(!exec->hadException()); Let's move this ASSERT to ~JSMainThreadExecState, to catch all cases.
Thanks for the review. The patch has been updated with the feedback, and landed in r167142: <http://trac.webkit.org/r167142>.