Execution of request.onreadystatechange crashes Firefox

VERIFIED FIXED in mozilla14

Status

()

Core
DOM
--
critical
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: Honza, Assigned: bz)

Tracking

({crash, regression})

Trunk
mozilla14
x86
Windows Vista
crash, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14-)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 612113 [details]
Firebug test build

This report is based on the following thread:
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/49c2d30c38c40504/81252a5b7b76b655?hl=en#81252a5b7b76b655


Firebug is overriding <xhr-request>.onreadystatechange and when the
callback is executed it calls the original handler, see here: 

https://github.com/firebug/firebug/blob/572bc4883aa895b2bcac53e555e6f6434aa11ec1/extension/content/firebug/net/spy.js#L819

onreadystatechange is now just a Function (since Fx14) so, it's executed using 'call':

var originalHandler = request.onreadystatechange;
originalHandler.call(request, event);   // <- causes crash

Execution of the second line causes Firefox crash.

Crash Report:
https://crash-stats.mozilla.com/report/index/bp-2fcfa090-a26b-43d6-bbcc-45aef2120403

The first Nightly where the bug appeared:
http://hg.mozilla.org/mozilla-central/rev/3e46009daea3

It still appears in today's Nightly:
http://hg.mozilla.org/mozilla-central/rev/c410b2d6d570

STR:

1) Install attached Firebug build
2) Enable the Console panel
3) Load www.google.com
4) Type something into the search box -> CRASH

Honza
Peter, are we possibly ending up returning the content function to chrome code without putting a cross-compartment wrapper on it?
Blocks: 740069
Severity: normal → critical
tracking-firefox14: --- → ?
Component: XPConnect → DOM
Keywords: crash, regression
QA Contact: xpconnect → general
Yup.  In a debug build, I get:

Assertion failure: compartment mismatched, at ../../../mozilla/js/src/jscntxtinlines.h:156

(gdb) frame 7
#7  0x135c8876 in js::CallJSNative (cx=0x22c529d0, native=0x16179ec0 <get_onreadystatechange>, args=@0xbfff65b4) at jscntxtinlines.h:316
316             assertSameCompartment(cx, args.rval());
Created attachment 612189 [details] [diff] [review]
Make sure to JS_WrapValue values being returned for callback types and 'any' types.
Attachment #612189 - Flags: review?(peterv)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
(Reporter)

Comment 4

6 years ago
Perhaps this one is related?
Bug 742631 - Crash [@ js::ContextStack::currentScriptWithDiagnostics(unsigned char**)] 

Honza
Comment on attachment 612189 [details] [diff] [review]
Make sure to JS_WrapValue values being returned for callback types and 'any' types.

Would be nice to have a test.
Attachment #612189 - Flags: review?(peterv) → review+
http://hg.mozilla.org/mozilla-central/rev/1907c8bf3cae with test.
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla14
Er, https://hg.mozilla.org/integration/mozilla-inbound/rev/1907c8bf3cae
https://hg.mozilla.org/mozilla-central/rev/1907c8bf3cae
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
tracking-firefox14: ? → -
(Reporter)

Comment 9

5 years ago
My STR from comment #0 is fixed in
http://hg.mozilla.org/mozilla-central/rev/9ca66ce2672f

Thanks!

Honza
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.