Closed Bug 830332 Opened 7 years ago Closed 7 years ago

jittest gc/incremental-state.js fails if rooting analysis enabled

Categories

(Core :: JavaScript Engine, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(1 file, 1 obsolete file)

Enabling rooting analysis by setting the JS_GC_ZEAL environment variable breaks any tests that attempt to set the GC zeal setting, as the environment variable takes precedence.

Currently this is only gc/incremental-state.js.
Attached patch Proposed fix (obsolete) — Splinter Review
The fix is to add a test metaline that disables the test if the environment variable is set.
Assignee: general → jcoppeard
Status: NEW → ASSIGNED
Duplicate of this bug: 830583
Attached patch Better fixSplinter Review
Instead of ignoring the test, allow use of gczeal() to override that set by the environment.  Tidy the initialization of zeal mode so that it happens inside js_InitGC().

Perform more sanity checking on the zeal value set from the environment variable.

Fix an assertion failure if js_InitGC() fails.

Fix a rooting bug in GlobalObject exposed by turning on rooting analysis sooner.
Attachment #701789 - Attachment is obsolete: true
Attachment #702285 - Flags: review?(wmccloskey)
Comment on attachment 702285 [details] [diff] [review]
Better fix

Review of attachment 702285 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsgc.cpp
@@ +907,5 @@
> +        return true;
> +
> +    int zeal = -1;
> +    int frequency = JS_DEFAULT_ZEAL_FREQ;
> +    if (strcmp(env, "help")) {

Could you add a != 0 at the end here?
Attachment #702285 - Flags: review?(wmccloskey) → review+
Blocks: 745742
(In reply to Bill McCloskey (:billm) from comment #4)

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b83a5ada45f
https://hg.mozilla.org/mozilla-central/rev/9b83a5ada45f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.