Closed Bug 829798 Opened 7 years ago Closed 7 years ago
Crash [@ Weak
Map _set _impl] or [@ Weak Map _set] or "Assertion failure: cx->runtime->preserve Wrapper Callback,"
(new WeakMap).set(FakeDOMObject.prototype, this) asserts js debug shell on m-c changeset 44dcffe8792b without any CLI arguments at Assertion failure: cx->runtime->preserveWrapperCallback, and crashes js opt shell at WeakMap_set_impl with WeakMap_set on the stack. This seems to be a null deref, but setting s-s just-in-case. autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 118460:945ed4328628 user: Andrew McCreight date: Fri Nov 09 10:59:02 2012 -0800 summary: Bug 777385 - Support (some) Paris bindings objects as weak map keys. r=peterv Setting to track for 20 and 21, though the regressing bug hasn't landed on aurora 20 yet, it will probably be landed soon, and the fix for this bug should also land on that branch.
This is a shell-only null-deref, so I'm unhiding. We only trigger the callback if we detect a wrapped native, and I was implicitly relying on that to check that we're in the browser, and thus have defined the callback. Apparently the shell has a FakeDOMObject that passes the check for being a new DOM bindings object, so we hit the callback. The question is, what is the right thing to do here? On one hand, we could define a fake callback, but what should that even do? On the other hand, we could add a NULL check, but it seems lame to dynamically check a static property. On the third hand, who cares about the speed of a null check.
Assignee: general → continuation
Shell only, doesn't really need to be tracked.
Gary said that jandem and bz added this fake DOM object to the shell. Do either of you have any thoughts on what would be appropriate here? How "realistic" is FakeDOMObject?
(In reply to Andrew McCreight [:mccr8] from comment #3) > Gary said that jandem and bz added this fake DOM object to the shell. Do > either of you have any thoughts on what would be appropriate here? How > "realistic" is FakeDOMObject? FakeDOMObject was added in bug 822385.
It's just realistic enough to trigger JIT optimizations so far. But we can make it more realistic as needed, no? The real issue here is the lack of a preserveWrapperCallback in the shell, while the weakmap code assumes that such things should exist, right? We could just set up a no-op callback in the shell if desired.
Still need to add a test.
Attachment #702400 - Flags: review?(wmccloskey) → review+
Crash Signature: [@ WeakMap_set_impl] [@ WeakMap_set] → [@ WeakMap_set_impl] [@ WeakMap_set]
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unknown exception (check manually)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 702400 [details] [diff] [review] add preserve wrapper to the shell that always returns true [Approval Request Comment] This patch is needed to allow fuzzing with bug 777385. It only affects the JS shell, so it is not part of the build. Bug caused by (feature/regressing bug #): 777385 User impact if declined: nothing direct, it just gets in the way of fuzzing Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): not part of the build String or UUID changes made by this patch: none
Attachment #702400 - Flags: approval-mozilla-aurora?
Comment on attachment 702400 [details] [diff] [review] add preserve wrapper to the shell that always returns true approved nptob.
Attachment #702400 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.