Closed
Bug 674597
Opened 13 years ago
Closed 13 years ago
abort if attempting to create an xpcom proxy for wrapped JS
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 675221
mozilla8
People
(Reporter: luke, Assigned: luke)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
With bug 667535 and bug 674571 fixed (and maybe a few more), we shouldn't be creating xpcom proxies for wrapped JS. Doing so violates a lot of preconditions. This bug is to create a (release mode) check and abort if this actually occurs in the wild (extensions). bsmedberg is trying to remove xpcom proxies altogether, so this is really just a safety check in the interim.
Updated•13 years ago
|
Assignee | ||
Comment 1•13 years ago
|
||
On second thought, abort() may be a bit severe.
Attachment #548917 -
Flags: review?(benjamin)
Updated•13 years ago
|
Attachment #548917 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 2•13 years ago
|
||
Try server has revealed two more cases of proxied wrapped JS (one an nsIPrompt, I forget the other), but both were from main thread to main thread, so I just weakened the check.
Attachment #548917 -
Attachment is obsolete: true
Attachment #549220 -
Flags: review?(benjamin)
Updated•13 years ago
|
Attachment #549220 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 3•13 years ago
|
||
Whiteboard: [inbound]
Comment 4•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Version: unspecified → Trunk
Comment 5•13 years ago
|
||
If I understand this correctly, add-ons shouldn't be using GetProxyForObject at all, right?
Assignee | ||
Comment 6•13 years ago
|
||
They might, but xpcom/proxy's days are numbered; bsmedberg wants to remove it entirely.
Comment 7•13 years ago
|
||
Some add-ons are using it right now. The question is whether we should flag this in the compatibility bump code, and under which circumstances. It would also be good to know how that code can be changed so that it doesn't use proxies.
Assignee | ||
Comment 8•13 years ago
|
||
The abort will hit if the GetProxyForObject is called to proxy wrapped JS (that is, an IDL interface implemented in JS) to some other thread. This has always been unsafe since it runs the wrapped JS's scripted QI immediately (instead of on the destination thread); this bug just catches it eagerly.
Assignee | ||
Comment 9•13 years ago
|
||
Backed out to fix bug 678440 and a few other intermittent crashes:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5f0596a0b81e
See bug 674571 comment 11 for details.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 10•13 years ago
|
||
Shouldn't this also get backed out of aurora (once landed on central)? As these events were either side of the merge?
Assignee | ||
Comment 11•13 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #10)
Already done.
Comment 12•13 years ago
|
||
AFAICT the backout in comment #9 above fixed bug 678637 too. See also bug 678637 comment #17 for one easy test ("can I start ChatZilla over SSL?") to run the next time a patch lands on this bug.
Assignee | ||
Comment 13•13 years ago
|
||
Thanks for pointing that out Tony. The fix that will enable the patches to reland (bug 675221) should nuke all possibility of this problem, but we'll make sure to retest.
Assignee | ||
Updated•13 years ago
|
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•