468758 - [FIX]"ASSERTION: Why are we being called with a pending exception?" with img onload

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Why are we being called with a pending exception?: '!::JS_IsExceptionPending(mContext)', file /Users/jruderman/central/dom/src/base/nsJSEnvironment.cpp, line 1825

Comment 1

[Jesse Ruderman]

This testcase is timing-dependent in ugly ways.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.  I only get this when reloading the testcase.


Looking at the testcase, we're asserting while compiling the onload handler before dispatching the event to it.  But the only place exceptions here come from is the onload handler, and we do clear the exception after that runs...

brendan, any idea how we might be ending up with an uncleared exception here?

Comment 3

[Brendan Eich [:brendan]]

What is cx->exception? What hex jsval value? If object, what is js_DumpObject(cx->exception)?

If you can break just before the script that might be leaving a pending exception, you could try watching cx->throwing (gdb watchpoint).

/be

Comment 4

[Boris Zbarsky [:bzbarsky]]

(gdb) p/x mContext->exception
$4 = 0xa2ca2c0
(gdb) call js_DumpObject(mContext->exception)
object 0xa2ca2c0
class 0xdb4ec0 XPCWrappedNative_NoHelper
no own properties - see proto (XPC_WN_NoMods_NoCall_Proto_JSClass at 0xa2ca2a0)
slots:
   0 (proto) = <XPC_WN_NoMods_NoCall_Proto_JSClass object at 0xa2ca2a0>
   1 (parent) = <Window object at 0x9605b60>
   2 (private) = 0xbd82760

It's wrapping an nsXPCException.  That nsXPCException has "unexpected error" as the mMessage, and NS_ERROR_UNEXPECTED as mResult.

The stack to that exception object being created is:

#0  nsXPCException::nsXPCException (this=0xc5b3cc0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcexception.cpp:143
#1  0x00d28f4d in nsXPCException::NewException (aMessage=0xdaae6c "Unexpected error", aResult=2147549183, aLocation=0x0, aData=0x0, exceptn=0xbfffc494) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcexception.cpp:439
#2  0x00d38c13 in XPCThrower::BuildAndThrowException (cx=0x12ebe00, rv=2147549183, sz=0xdaae6c "Unexpected error") at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcthrower.cpp:231
#3  0x00d38e5c in XPCThrower::Throw (rv=2147549183, cx=0x12ebe00) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcthrower.cpp:56
#4  0x00d52cf2 in Throw (errNum=2147549183, cx=0x12ebe00) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:54
#5  0x00d55ea1 in XPC_WN_Helper_AddProperty (cx=0x12ebe00, obj=0xbcec140, idval=47639316, vp=0xbfffc63c) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:939
#6  0x002f26d1 in js_DefineNativeProperty (cx=0x12ebe00, obj=0xbcec140, id=47639316, value=113936544, getter=0xd55bf2 <XPC_WN_Helper_GetProperty>, setter=0xd55b06 <XPC_WN_Helper_SetProperty>, attrs=5, flags=0, shortid=0, propp=0x0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsobj.cpp:3259
#7  0x002f2c02 in js_DefineProperty (cx=0x12ebe00, obj=0xbcec140, id=47639316, value=113936544, getter=0, setter=0, attrs=5, propp=0x0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsobj.cpp:3141
#8  0x002502b2 in DefinePropertyById (cx=0x12ebe00, obj=0xbcec140, id=47639316, value=113936544, getter=0, setter=0, attrs=5, flags=0, tinyid=0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsapi.cpp:3135
#9  0x002503b5 in DefineProperty (cx=0x12ebe00, obj=0xbcec140, name=0x3d7a69e "onload", value=113936544, getter=0, setter=0, attrs=5, flags=0, tinyid=0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsapi.cpp:3158
#10 0x00250918 in JS_DefineProperty (cx=0x12ebe00, obj=0xbcec140, name=0x3d7a69e "onload", value=113936544, getter=0, setter=0, attrs=5) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsapi.cpp:3247
#11 0x039d2fc7 in nsJSContext::BindCompiledEventHandler (this=0xd5d46c0, aTarget=0xc54eb80, aScope=0xa2c7040, aName=0x1168d58, aHandler=0x6c7b188) at /Users/bzbarsky/mozilla/debug/mozilla/dom/src/base/nsJSEnvironment.cpp:2067
#12 0x037e2ed0 in nsEventListenerManager::CompileEventHandlerInternal (this=0xd5eee10, aContext=0xd5d46c0, aScope=0xa2c7040, aObject=0xc54eb80, aName=0x1168d58, aListenerStruct=0xd5eee38, aCurrentTarget=0xc54eb80) at /Users/bzbarsky/mozilla/debug/mozilla/content/events/src/nsEventListenerManager.cpp:1044

Looks like nsEventReceiverSH::SetProperty fails to get a script context by calling nsJSUtils::GetStaticScriptContext and returns an error.  XPConnect then tosses us into the stack above.

So is the caller of JS_DefineProperty supposed to handle reporting errors after it returns or something?  Note that there's no script running when this stack happens; it's all JSAPI access.

Comment 5

[Brendan Eich [:brendan]]

Yes, JS API users who can't count on propagating a pending exception back to a script (where it could be caught, and is guaranteed to be reported if not caught when control unwinds from the JS_Execute* or JS_Evaluate* API call) must in fact report or clear the pending exception.

/be

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK.  Sounds like BindCompiledEventHandler should do that, I would say.  jst, what do you think?

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fwiw, that sounds right to me.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 352763 [details] [diff] [review]
Like so

Looks right to me. r+sr=jst

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/d41063000f1e

Not sure how to write a reliable test for this.  :(

I'm tempted to say we don't need this on branch, but we could take it if desired.