Closed Bug 390918 Opened 17 years ago Closed 17 years ago

"Assertion failure: !gen->frame.down" with gc in generator

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: igor)

References

Details

(Keywords: assertion, regression, testcase)

Attachments

(1 file, 2 obsolete files)

jsfunfuzz is hitting this every few seconds on trunk:

js> for (i in (function(){ gc(); yield null; })());
Assertion failure: !gen->frame.down, at jsiter.c:684
Flags: blocking1.9?
 This is caused by my bogus assertion in the patch for bug 390078.
Assignee: general → igor
Blocks: 390078
The reason for the regression is that I forgot that generator's frame became a part of that stack when the generator run. But that also points that the current code does unnecessary marking in that case. When the the frame is active, js_TraceContext traces it as aprt of the JS stack.
I filed bug 390950 about unnecessary tracing.
Here is an alternative test case with better coverage as now it tests for a generator which is being closed.

function gen()
{
    gc();
    try {
        yield 1;
    } finally {
        gc();
    }
}

var iter = gen();
for (var i in iter)
    iter.close();

print("OK");
Status: NEW → ASSIGNED
Attached patch fix v1 (obsolete) — Splinter Review
Attachment #275264 - Flags: review?(brendan)
Attachment #275264 - Flags: approval1.9?
Attached patch fix v1b (obsolete) — Splinter Review
a new version with better comments
Attachment #275264 - Attachment is obsolete: true
Attachment #275266 - Flags: review?(brendan)
Attachment #275266 - Flags: approval1.9?
Attachment #275264 - Flags: review?(brendan)
Attachment #275264 - Flags: approval1.9?
Comment on attachment 275266 [details] [diff] [review]
fix v1b

>+        /*
>+         * js_TraceStackFrame does not trace recursively the down chain so we

Suggest "does not recursively trace the down-linked frame chain, so we ...".

>+         * insist that gen->frame has no parent to trace when the generator
>+         * is not running.
>+         */
>+        JS_ASSERT_IF(gen->state != JSGEN_RUNNING &&
>+                     gen->state != JSGEN_CLOSING,
>+                     !gen->frame.down);
>+
>+        /*
>+         * FIXME be 390950. Generator's frame is a part of the JS stack when
>+         * the generator is running or closing. Thus tracing the frame in this
>+         * case here duplicates the work done in js_TraceContext.
>+         */
>         js_TraceStackFrame(trc, &gen->frame);
>     }

For future reference, this function should invert the test to return early if (!gen) and avoid over-indenting common code.

/be
Attachment #275266 - Flags: review?(brendan)
Attachment #275266 - Flags: review+
Attachment #275266 - Flags: approval1.9?
Attachment #275266 - Flags: approval1.9+
Attached patch fix v1cSplinter Review
fix with nits addressed
Attachment #275266 - Attachment is obsolete: true
Attachment #275298 - Flags: review+
Attachment #275298 - Flags: approval1.9+
Here is modified test from comment 4 to test the generator behavior more throughly :

function gen()
{
    var c = [1, "x"];
    gc();
    try {
        yield c;
    } finally {
        gc();
    }
}

var iter = gen();
var i;
for (i in iter) {
    gc();
    iter.close();
}

if (!(i.length === 2 && i[0] === 1 && i[1] === "x"))
    throw "Unexpected yield result: "+i;
I checked in the patch from comment 8 to the trunk:

Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v  <--  jsiter.c
new revision: 3.70; previous revision: 3.69
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Flags: blocking1.9?
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-390918.js,v  <--  regress-390918.js
initial revision: 1.1
Flags: in-testsuite+
verified fixed 1.9.0 linux/mac*/windows
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: