Closed Bug 295606 Opened 17 years ago Closed 17 years ago

Reentry of JS_ResolveStandardClass due to fix for bug 289675

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta3

People

(Reporter: bzbarsky, Assigned: brendan)

References

Details

(Keywords: js1.5)

Attachments

(1 file)

STEPS TO REPRODUCE:

1)  Start build
2)  Breakpoint in 'js_InitQNameClass'
3)  Open a new window
4)  Continue from breakpoint.

Note that now you're back at the breakpoint.  The stack to get here looks
something like (leaving out some parts for clarity):

#0  0xb7fe5f7c in js_InitQNameClass (cx=0x89076e8, obj=0x85f29d8)
    at ../../../mozilla/js/src/jsxml.c:7340
#1  0xb7f3b039 in JS_ResolveStandardClass (cx=0x89076e8, obj=0x85f29d8,
id=135445964, 
    resolved=0xbfff9758) at ../../../mozilla/js/src/jsapi.c:1425
#2  0xb6ecf24f in nsWindowSH::NewResolve (this=0x8510b70, wrapper=0x8a37870, 
    cx=0x89076e8, obj=0x85f29d8, id=135445964, flags=16, objp=0xbfff9810, 
    _retval=0xbfff9814) at ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:5083
#3  0xb79a61e4 in XPC_WN_Helper_NewResolve (cx=0x89076e8, obj=0x85f29d8, 
    idval=135445964, flags=16, objp=0xbfff9954)
    at ../../../../../mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:966
#4  0xb7f9b5b1 in js_LookupPropertyWithFlags (cx=0x89076e8, obj=0x85f29d8,
id=135527808, 
    flags=16, objp=0xbfff99d4, propp=0xbfff99d0) at
../../../mozilla/js/src/jsobj.c:2542
#5  0xb7f99cfa in js_FindConstructor (cx=0x89076e8, start=0x0, 
    name=0xb7ff9a68 "AttributeName", vp=0xbfff9a1c)
    at ../../../mozilla/js/src/jsobj.c:1955
#6  0xb7f9f65b in GetClassPrototype (cx=0x89076e8, scope=0x85f29d8, 
    name=0xb7ff9a68 "AttributeName", protop=0xbfff9aa4)
    at ../../../mozilla/js/src/jsobj.c:3643
#7  0xb7f99663 in js_NewObject (cx=0x89076e8, clasp=0xb7fff480, proto=0x0, 
    parent=0x85f29d8) at ../../../mozilla/js/src/jsobj.c:1836
#8  0xb7f3bd74 in JS_InitClass (cx=0x89076e8, obj=0x85f29d8, parent_proto=0x0, 
    clasp=0xb7fff480, constructor=0xb7fd4f17 <AttributeName>, nargs=2,
ps=0xb7fff540, 
    fs=0xb7fff570, static_ps=0x0, static_fs=0x0) at
../../../mozilla/js/src/jsapi.c:1955
[...]
#11 0xb7f3ac48 in JS_InitStandardClasses (cx=0x89076e8, obj=0x85f29d8)
    at ../../../mozilla/js/src/jsapi.c:1182
#12 0xb795b37d in nsXPConnect::InitClassesWithNewWrappedGlobal (this=0x8128b88, 

Note that since js_InitQNameClass inits three classes, we'll reenter it three
times, so we'll init each of those classes three times.

Is that really desirable behavior?  It seems pretty wacky to me...
Blocks: 289675
Flags: blocking1.8b3?
Note that BackstagePass::NewResolve leads to similar code flow...
I have a fix for this already under construction for bug 292903.

/be
Assignee: jst → brendan
Depends on: 292903
Keywords: js1.5
Priority: -- → P2
QA Contact: ian → general
Target Milestone: --- → mozilla1.8beta3
Waitaminute -- why are we using both JS_InitStandardClasses (frame 11) and
JS_ResolveStandardClass (frame 1)?  One or the other, not both?

I am fixing js_InitQNameClass over in but 292903, but there seems to be a
separate bug here.  jst, didn't we at one point have no JS_InitStandardClasses
calls when (re-)initializing a window object, and only the
JS_ResolveStandardClass call in nsWindowSH::NewResolve?

/be
bz: what was frame #13?

/be
> Waitaminute -- why are we using both 

One's XPConnect initing a new global object, while the other is the resolve hook
in nsWindowSH classinfo.

> and only the JS_ResolveStandardClass call in nsWindowSH::NewResolve?

Right, but class init ends up resolving on the global, which calls into
nsWindowSH::NewResolve.  See stack.

> bz: what was frame #13?

There are two varieties of frame #13.  nsJSContext::InitContext and
mozJSComponentLoader::GlobalForLocation.  The former happens any time a new
window is created; the latter at startup.
(In reply to comment #5)

> > and only the JS_ResolveStandardClass call in nsWindowSH::NewResolve?
> 
> Right, but class init ends up resolving on the global, which calls into
> nsWindowSH::NewResolve.  See stack.

I know, but that's a bug (if not *the* bug).  If we're lazily resolving standard
classes, we must not call JS_InitStandardClasses.  If we are eagerly defining
standard classes, we must call JS_InitStandardClasses but not call
JS_ResolveStandardClass from our global class's resolve hook.  The JS APIs and
underlying engine code can't work with both eager and lazy code paths operating
for a given global.

> > bz: what was frame #13?
> 
> There are two varieties of frame #13.  nsJSContext::InitContext and
> mozJSComponentLoader::GlobalForLocation.  The former happens any time a new
> window is created; the latter at startup.

But neither calls JS_InitStandardClasses directly.  Only if the
nsIXPConnect::INIT_JS_STANDARD_CLASSES flag is passed into
nsIXPConnect::InitClassesWithNewWrappedGlobal would that method call
JS_InitStandardClasses.  I'm still having double-auto-reg woes, but I will try
to gdb in a bit.  If anyone could say what frame 13 was exactly (and frame 14,
given the layering), that would be swell.

/be
(In reply to comment #6)
> (In reply to comment #5)
> > There are two varieties of frame #13.  nsJSContext::InitContext and
> > mozJSComponentLoader::GlobalForLocation.  The former happens any time a new
> > window is created; the latter at startup.
> 
> But neither calls JS_InitStandardClasses directly. 

Duh, sorry -- pre-caffeine here.  I was off by one in my frame memory and didn't
even look at the stack in comment 0.

Still, unless someone is wrongly passing nsIXPConnect::INIT_JS_STANDARD_CLASSES
to nsIXPConnect::InitClassesWithNewWrappedGlobal, I don't see how that method
can call JS_InitStandardClasses.

/be
I'm going to check in with r+sr=jst.

/be
Attachment #186333 - Flags: superreview+
Attachment #186333 - Flags: review+
Attachment #186333 - Flags: approval1.8b3+
Checked in.  I'll fix the rest in 292903.

/be
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
bug 289675 landed on the aviary branch -- do we need this fix there as well?
Flags: blocking1.7.9?
Flags: blocking-aviary1.0.5?
No, based on that patch I think that the XPCNativeWrapper changes were needed in
addition to bug 289675.  Brendan, correct me if I'm wrong please!
We didn't land any of the flag-system-object stuff on the branch.

/be
Flags: blocking1.8b3?
Flags: blocking1.7.9?
Flags: blocking-aviary1.0.5?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.