Closed
Bug 336054
Opened 19 years ago
Closed 19 years ago
"Assertion failure: !OBJ_GET_PROTO(cx, ctor)" in JS_InitClass, involving Greasemonkey
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: brendan)
References
Details
(Keywords: assertion, crash, regression)
Attachments
(2 files, 4 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
Steps to reproduce (I think):
1. Install Greasemonkey
2. Install http://www.squarefree.com/userscripts/bug-attachment-source.user.js
3. Load any bug (e.g. bug 12345) in a debug build.
Result:
Assertion failure: !OBJ_GET_PROTO(cx, ctor), at /Users/admin/trunk/mozilla/js/src/jsapi.c:2177
Program received signal SIGABRT, Aborted.
Stack to follow. Shaver and mrbkap are already on it (discussed in #content just now).
Reporter | ||
Comment 1•19 years ago
|
||
The bug is that we're setting sandcx's global object to be one that has already had standard classes initialized against it, but sandcx->classObjects is of course still all NULLs: we initalized the classes against this global using another context!
I think this pattern needs to be supported, since we advocate context pooling, but I'm not sure the best way to get sandcx->classObjects refilled appropriately. Filling from the global object in the "obvious" way could result in us treating a script-set "Function" (f.e.) as the canonical/original Function constructor, which we take pains to avoid for ECMA compliance and performance.
Brendan?
This is a regression from 304376.
Blocks: 304376
Keywords: regression
Reporter | ||
Updated•19 years ago
|
Summary: "Assertion failure: !OBJ_GET_PROTO(cx, ctor)" in JS_InitClass → "Assertion failure: !OBJ_GET_PROTO(cx, ctor)" in JS_InitClass, involving Greasemonkey
Assignee | ||
Comment 4•19 years ago
|
||
Attachment #220449 -
Flags: superreview?(shaver)
Attachment #220449 -
Flags: review?(mrbkap)
Assignee | ||
Comment 5•19 years ago
|
||
Assignee | ||
Comment 6•19 years ago
|
||
Assignee | ||
Updated•19 years ago
|
Attachment #220458 -
Attachment is obsolete: true
Comment on attachment 220449 [details] [diff] [review]
proposed fix
As Jesse found the hard way, this doesn't really handle the setting of global objects to NULL very well. :) sr-=shaver, likely a trivial fix but I haven't really thought through the implications of just NULL-checking obj.
Attachment #220449 -
Flags: superreview?(shaver)
Attachment #220449 -
Flags: superreview-
Attachment #220449 -
Flags: review?(mrbkap)
Assignee | ||
Comment 8•19 years ago
|
||
There's nothing profound about a null-test. It's simply necessary when clearing the global object, which is done via JS_SetGlobalObject(cx, NULL). This patch will clear cx->classObjects in that case.
/be
Assignee: general → brendan
Attachment #220449 -
Attachment is obsolete: true
Attachment #220470 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #220479 -
Flags: superreview?(shaver)
Attachment #220479 -
Flags: review?(mrbkap)
Comment on attachment 220479 [details] [diff] [review]
combined js library and shell patch
sr=shaver
Attachment #220479 -
Flags: superreview?(shaver) → superreview+
Assignee | ||
Comment 10•19 years ago
|
||
Picked nit in js.c: set ok to JS API return value where possible.
/be
Attachment #220483 -
Flags: review?(mrbkap)
Assignee | ||
Updated•19 years ago
|
Attachment #220479 -
Attachment is obsolete: true
Attachment #220479 -
Flags: review?(mrbkap)
Assignee | ||
Comment 11•19 years ago
|
||
Fixed, mrbkap feel free to review -- I'll pick any nits tomorrow.
/be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 12•19 years ago
|
||
Comment on attachment 220483 [details] [diff] [review]
patch I'm checking in
Cool.
Attachment #220483 -
Flags: review?(mrbkap) → review+
Updated•19 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•