Closed
Bug 570169
Opened 14 years ago
Closed 8 years ago
Add assertions that gcthings do not leak across compartments
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(3 files)
15.67 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
55.37 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
1.82 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
Initially, the assertions will flunk all over the place, so I'll make them just print a warning instead of the usual fatal JS_ASSERT. We'll toughen them up later.
I doubt that this blocks bug 563009
Assignee | ||
Comment 2•14 years ago
|
||
This is to avoid circular dependencies once part 2 adds a bunch of new assertion stuff in jscntxtinlines.h.
Assignee: general → jorendorff
Attachment #451671 -
Flags: review?(gal)
Assignee | ||
Comment 3•14 years ago
|
||
They are not hard assertions. Right now they just print a line of noise to stdout: *** Compartment mismatch 0x838fa70 vs. 0x8381898
Attachment #451673 -
Flags: review?(gal)
Assignee | ||
Comment 4•14 years ago
|
||
This isn't exactly pretty, but it's the shortest path, it won't hurt perf much, and we will clean it up later.
Attachment #451675 -
Flags: review?(gal)
Comment 5•14 years ago
|
||
Comment on attachment 451673 [details] [diff] [review] Part 2, add assertions - v1 The capital ASSERT functions are a bit surprising and look like macros but we do that elsewhere too. Not vouching for completeness. XPConnect does reach into the engine's guts occasionally.
Attachment #451673 -
Flags: review?(gal) → review+
Updated•14 years ago
|
Attachment #451671 -
Flags: review?(gal) → review+
Comment 6•14 years ago
|
||
Comment on attachment 451675 [details] [diff] [review] Part 3, cope with E4X and compiler-internal objects - v1 E4X, have you no shame?
Attachment #451675 -
Flags: review?(gal) → review+
Comment 7•14 years ago
|
||
(In reply to comment #6) > (From update of attachment 451675 [details] [diff] [review]) > E4X, have you no shame? There's nothing shameful in the default compartment :-P. Jason, do you prefer cls to clasp as canonical JSClass * variable name? We've used clasp for a long time. We had a contributor (first autoconf build system contributor), Chris Seawood, whose nick/username was cls, so I shied away from that. But it's a bit drop-the-vowels cyber-cruddy too. /be
Assignee | ||
Comment 8•14 years ago
|
||
Sorry, I'll fix it. I do prefer cls to clasp, as a personal foible, but the convention is really strong.
Assignee | ||
Comment 9•14 years ago
|
||
This was briefly landed but I backed it out. For one thing, I forgot that JS_NewArrayObject allows a NULL vector argument with a nonzero length argument. So it turned everything involving debug builds orange. I'm running the updated patch through the try server now.
Comment 10•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/bdea0d92907c
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 12•14 years ago
|
||
Jason whats the status here?
Comment 13•10 years ago
|
||
jorendorff: are your r+'d patches (from 201) adding compartment leak asserts still relevant?
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 14•8 years ago
|
||
No.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 8 years ago
Flags: needinfo?(jorendorff)
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•