Closed Bug 387871 Opened 14 years ago Closed 14 years ago

"Assertion failure: gen->state != JSGEN_RUNNING && gen->state != JSGEN_CLOSING"

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: igor)

References

Details

(Keywords: assertion, testcase)

Attachments

(1 file)

js> a = (1 for each (x in a)); for (i in a) { }
Assertion failure: gen->state != JSGEN_RUNNING && gen->state != JSGEN_CLOSING, at jsiter.c:923

In opt, I get:
typein:1: TypeError: already executing generator ({})
Assignee: general → igor
This is a regression from bug 349326. The changes to ensure that for-in loop always closes iterator affected ClodeGenerator from jsiter.c so that it can be called for a generator in an any state, like in the following more explicit example demonstrates:

var a = gen();

try {
    a.next();
    throw "a.next() should throw about already invoked generator";
} catch (e) {
    if (!(e instanceof TypeError))
        throw e;
}

function gen()
{
    for (x in a)
        yield 1;
}

Since "a" here is a generator instance, "for (x in a)" loop translates into:

try {
    for (;;) {
        x = a.next();
        yield 1;
    }
} catch (e if StopIteration) { 
} finally {
    a.close();
}

The first a.next() call correctly throws already running exception. Then a.close() is called (this is what bug 349326 added). It should also throw already running exception but due to the bug we got an assert.
Blocks: 349326
Status: NEW → ASSIGNED
Attached patch fix v1Splinter Review
Comment on attachment 272544 [details] [diff] [review]
fix v1

The patch moves dealing with recursive invocation of generators to SendToGenerator to share the code with CloseGenerator.
Attachment #272544 - Attachment is patch: true
Attachment #272544 - Attachment mime type: text/x-patch → text/plain
Attachment #272544 - Flags: review?(brendan)
Comment on attachment 272544 [details] [diff] [review]
fix v1

Random logic (open coded) state machine read via context diff is not my favorite review ;-). But I looked at full file context and it's good. Thanks,

/be
Attachment #272544 - Flags: review?(brendan) → review+
I checked in the patch from comment 3 to the trunk:

Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v  <--  jsiter.c
new revision: 3.65; previous revision: 3.64
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-387871.js,v  <--  regress-387871.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.