Closed Bug 612225 Opened 9 years ago Closed 9 years ago

Assertion failure: compartment mismatched,

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: Dolske, Assigned: luke)

References

Details

(Whiteboard: hardblocker, fixed-in-tracemonkey)

Attachments

(4 files, 1 obsolete file)

Attached patch Testcase patchSplinter Review
Probably a dupe of bug 606138 / bug 610941 / bug 603524, but filing because it's easier to reproduce.

1) Apply test patch attached here, from m-c changeset cd3babdf6a5e
2) In objdir, run python _tests/testing/mochitest/runtests.py --test-path=toolkit/components/passwordmgr
3) click test_prompt.html
4) instacrash
Adding "netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');" at the top of my observer makes the crash/assert go away, fwiw.
We should fix this for b8. Can someone block on it please?
blocking2.0: --- → ?
Target Milestone: --- → mozilla2.0b8
blocking2.0: ? → beta9+
Needs an assignee.
Assignee: general → gal
Assignee: gal → nobody
Component: JavaScript Engine → XPConnect
OS: Mac OS X → All
QA Contact: general → xpconnect
Hardware: x86 → All
Stealing with permission.
Assignee: gal → lw
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
blocking2.0: beta9+ → betaN+
Fixing fields my automated script accidentally blanked. Apologies for the bugspam
Attached patch reduced testcaseSplinter Review
This reduced testcase produces the mismatch assertion.

If you break at math_cos in the testcase cx->throwing == true so a (hopefully the) bug seems to be an exception getting dropped.  The bug exists in the exit path
  XPWN::FindTearOff -> nsJSIID::HasInstance -> XPC_WN_Helper_HasInstance.

The FindTearOff interface makes error reporting optional which is just asking for trouble.  We should probably change the signature and check all callers.
Attached patch basic patch (obsolete) — Splinter Review
This patch just propagates the error (fixing a JSBool -> nsresult conversion and pulling out non-error errors so that mochitests doesn't fall over) which fixes the reduced and initial testcase.

A better fix would be to change the interface of FindTearOff so that it wouldn't be so easy to drop errors.
Attachment #501479 - Flags: review?(mrbkap)
Attached patch better fixSplinter Review
That last patch would have stopped reporting NS_ERROR_NO_INTERFACE.
Attachment #501479 - Attachment is obsolete: true
Attachment #501527 - Flags: review?(mrbkap)
Attachment #501479 - Flags: review?(mrbkap)
Attached patch another fixSplinter Review
I found another place where FindTearOff's return value was ignored.
Attachment #501531 - Flags: review?(mrbkap)
Green on try.
Attachment #501527 - Flags: review?(mrbkap)
Attachment #501527 - Flags: review+
Attachment #501531 - Flags: review?(mrbkap)
Attachment #501531 - Flags: review+
Looks good to me. I am a bit fuzzy on why our tests treat NS_ERROR_NO_INTERFACE as not a failure, so I am leaving a ? up for mbkap for final judgement.
http://hg.mozilla.org/tracemonkey/rev/daaf6a3b8782
Whiteboard: hardblocker, fixed-in-tracemonkey
Comment on attachment 501527 [details] [diff] [review]
better fix

NO_INTERFACE is what QueryInterface "throws" for an interface the object doesn't support. It isn't a fatal error, just an indication that the object you got back was null. So we squash it in places where we're calling QI on an object and don't know whether or not it actually supports the interface we're passing in.
Attachment #501527 - Flags: review?(mrbkap) → review+
Attachment #501531 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/mozilla-central/rev/daaf6a3b8782
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.