281336 - Crash with <![CDATA[[

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

URL is a diff that causes mozilla to crash with a failed _CrtIsValidHeapPointer
assert.  If you replace:
+          <![CDATA[[
with
+          <![CDATA[
it does not crash.  I could not create a testcase that reproduced the crash.

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: stack

Comment 2

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050206

I couldn´t crash using this Mozilla Suite or FirefoxTrunk Build ID 2005012912,
but at first test seems I´ve been using the wrong testcase, from another tab, so
I wrongly commented a Talkback belonging to this bug.
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB3540437Q
Seems Talkback above belongs to Bug 281333 Crash [@ nsFrameItems::AddChild ] 

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Taking and confirming. This is the opposite problem of what Boris fixed in bug
280089. We had successfully compiled the getter, but the setter's compilation
failed, so when Destroy() was called with aIsCompiled == PR_FALSE, we tried
using 'delete' on the JS object, which caused MSVC's delete to assert. This
probably wouldn't actually crash on a non-debug build (depending on how delete
works), which may be why Hermann could not reproduce this.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Ah, fun.  So if the setter compilation fails we should probably delete the
getter ourselves....

Comment 5

[Boris Zbarsky [:bzbarsky]]

Except in this case we have no getter, do we?  So what gives?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment 4)
We could do that. Or, in Destroy(), we could inspect (mFlags & JSOP_GETTER) and
(mFlags & JSOP_SETTER), since those appear to be set in the right places. I
think I prefer that solution, but I think you technically own this code, so it's
your call ;-).

(In reply to comment 5)
That threw me off too. However, note
+      <property name="draggable" onget="return this._draggable;">
from the patch, has the 'onget' property.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #6)
> We could do that. Or, in Destroy(), we could inspect (mFlags & JSOP_GETTER) and
> (mFlags & JSOP_SETTER), since those appear to be set in the right places. I

Of course, I mean JSPROP_GETTER and JSPROP_SETTER. (sorry for the spam)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hmm... Yeah, checking the getter and setter flags seems reasonable.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

Basically, we make sure to use two flags for two non-dependant properties
instead of one.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173654 [details] [diff] [review]
patch v1

r+sr=bzbarsky

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.