Proposal: generator.close without GeneratorExit

VERIFIED FIXED in mozilla1.8.1

Status

()

defect
VERIFIED FIXED
13 years ago
13 years ago

People

(Reporter: igor, Assigned: igor)

Tracking

({verified1.8.1})

Trunk
mozilla1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [baking until 8/29])

Attachments

(4 attachments, 8 obsolete attachments)

The generator protocol that SpiderMonkey implements follows Python 2.5c1 implementation that dictates that the close method applied to the generator should throw GeneratorExit exception at the point of the last yield. Thus effectively iter.close() becomes a shorthand for:

try {
	iter.throw(GeneratorExit);
} catch (e) {
    if (!(e instanceof generatorExit))
        report e in one way or another;
}

That leads to rather complex interactions between various parts of the iteration protocol and unintuitive behavior for a user. For example, consider the following example:

function gen() {
	try {
		yield 1;
	} catch (e){
		print("Unexpected exception: "+e);	
	}
} 

for (var i in  gen()) {
	break;
}
 
With the currently defined protocol this should print "Unexpected exception: ..."  since after the loop terminates GeneratorExit would be thrown. But this is counterintuitive compared with the break statement in normal loops when a script could not use try-catch to prevent break from transferring control.  

Thus I suggest to change the behavior of the close method. The proposed behavior for generator.close() is to transfer the control back to the generator and execute return; statement after the yield statement.

In this way all the necessary finally blocks would be executed while no catch would be triggered as with normal return. Moreover, normal rules about exception thrown from finally blocks would applies with such exceptions propagating to the caller. And if the caller is GC, then a standard diagnostic should be issued about uncaught exception.
This proposal of changing  close to trigger return also solves the following problem reported in 

http://sourceforge.net/tracker/index.php?func=detail&aid=1542308&group_id=5470&atid=105470:

regarding the following example:

def gen():
  while True:
    try:
      yield
    except:
      pass

Its translation to JS is:

function gen() {
    while (true) {
        try {
            yield;
        } catch (e) {
        }   
    }
}

This will loop infinitely when the close method would be called either directly or through GC since now JS throws an exception when yield is called. But with this proposal no such loops are possible since return can not be ignored. Yes, a finally statement can contain infinite loop itself, but that is no different than any other infinite loop bug.
I like this proposal, except that until now, finally clauses can be compiled into bytecode such that no runtime logic has to find and "gosub" a finally.  With this proposed change, the JSOP_YIELD case in the interpreter, or JITted future versions of it, would have to detect when close is calling and find and invoke finallys.

That seems like a big implementation constraint.

/be
(In reply to comment #2)
> That seems like a big implementation constraint.

One way to satisfy it in SpiderMonkey is to throw an uncatchable exception that is caught by close.  Uncatchable exception to be defined.  Still not clear how hard this is, but at least it would reuse the compiled-out finally logic -- it would require no code generation changes.

/be
Posted patch Proof of concept (obsolete) — Splinter Review
The patch changes interpreter to ignore catch blocks when searching for a handler for GeneratorExit exception. Initially I tried to implement this without altering JSTryNote with extra flag to distinguish catch/finally, but it seems the idea of checking if the handler bytecode starts with JSOP_FINALLY or not requires some work.

The patch does not eliminate  GeneratorExit and in fact it does not even touches jsiter.c. This is for the next iteration of the patch.
Assignee: general → igor.bukanov
Status: NEW → ASSIGNED
Attachment #235145 - Flags: review?
Comment on attachment 235145 [details] [diff] [review]
Proof of concept

The JSTryNote.catchStart for a finally indexes the first bytecode of the "rethrow sequence":

[setsp, exception, gosub, throw]

See jsemit.c where finallyCatch is set non-negative.  So you can't inspect bytecode to see JSOP_FINALLY, at least not easily.

If we have to go this route, I would much rather:

1.  Use a bit, not a whole word, in JSTryNote -- the high bit on catchStart could be used without adding any overhead to SCRIPT_FIND_CATCH_START in the "miss" case.

2.  Use a different macro from SCRIPT_FIND_CATCH_START (call it SCRIPT_FIND_FINALLY_START) if and only if closing -- that is, find the finally rethrow sequence and set the saved frame.gen.pc to that sequence's first bytecode, all from within js_CloseGeneratorObject.  Don't change js_Interpret at all.

3.  To prevent try-in-outer-finally from catching GeneratorExit, eliminate GeneratorExit altogether (later patch, or this one ;-), and use a distinct cx->throwing value (2, so JSTHROW_NONE = 0, JSTHROW_EXCEPTION = 1, JSTHROW_CLOSING = 2).  This will change js_Interpret after all, but it should be possible to avoid penalizing exception-handling common cases.

/be
Attachment #235145 - Flags: review? → review-
Thinking about this more, I am less in favor.  The point of GeneratorExit in PEP 342 is so that a well-behaved generator can take final action when closed, not just when any exception is thrown at the open generator.  While finally may be a good way to take final action, catching GeneratorExit works too and can be useful in its own right, with or without finally.

It would be good to hear from Python people more about real-world use cases for GeneratorExit.

/be
(In reply to comment #6)
> Thinking about this more, I am less in favor.  The point of GeneratorExit in
> PEP 342 is so that a well-behaved generator can take final action when closed,
> not just when any exception is thrown at the open generator.  While finally may
> be a good way to take final action, catching GeneratorExit works too and can be
> useful in its own right, with or without finally.
> 
> It would be good to hear from Python people more about real-world use cases for
> GeneratorExit.

Python people, in fact, do not mind such change, but it is too late for 2.5:
 
http://mail.python.org/pipermail/python-dev/2006-August/068429.html
http://mail.python.org/pipermail/python-dev/2006-August/068433.html
http://mail.python.org/pipermail/python-dev/2006-August/068444.html
(In reply to comment #6)
> The point of GeneratorExit in
> PEP 342 is so that a well-behaved generator can take final action when closed,
> not just when any exception is thrown at the open generator.

As it was pointed out on python-dev@python.org, even with GeneratorExit replaced by asynchronous return a generator still can perform close-only actions:

function gen() {
    var explicitClose = true;
    try {
        yield;
        explicitClose = false;
    } catch (e) {
        explicitClose = false;
        throw e;
    } finally {
        if (explicitClose)
            doExplicitCloseOnlyActions();
    }
}

Yes, this is few extra lines of code compared with GeneratorExit code, but this not different from finally that wants to perform special actions when executed in response to break/continue jumps.
Posted patch Implementation v1 (obsolete) — Splinter Review
Changes compared with the proof of concept:

1. The patch distinguish JSTryNote corresponding to finaly blocks by checking the second bytecode that follows JSOP_SETSP. For finally it is  JSOP_EXCEPTION while for catch it is JSOP_ENTERBLOCK. Thus there is no need to change JSTryNote. 

2. A special JSRuntime.areturnException value replaces GeneratorExit. This is the least intrusive change as changing throwing required touching too many places changing finally to restore throwing appropriately.    

3. Changes for jsiter.c assumes that the patch from bug 349320 is applied.
Attachment #235145 - Attachment is obsolete: true
Attachment #235217 - Flags: review?(brendan)
Posted patch Implementation v2 (obsolete) — Splinter Review
This version has extra changes to jsgc.c to avoid scheduling for later closing the generator that yields outside any try with a finally block.
Attachment #235217 - Attachment is obsolete: true
Attachment #235232 - Flags: review?(brendan)
Attachment #235217 - Flags: review?(brendan)
Comment on attachment 235232 [details] [diff] [review]
Implementation v2

This is getting good.  But why rt->areturnException (the a in the name is unclear to me too, but I mean why have it at all)?  There are lots of crypto-boolean jsvals, a la JSVAL_HOLE.  You might even use that one (but don't define it in any public header).

>+            if (cx->exception != rt->areturnException) {

JS_LIKELY around this condition, or will gcc predict the then path for an if/else?

>+
>+    JS_ASSERT(tn->catchStart != 0);
>+    do {
>+        if ((jsuword)(off - tn->start) < (jsuword)tn->length) {
>+            /*
>+             * Catch bytecode begines with:

Fix "begines" typo.

>+             * JSOP_SETSP JSOP_ENTERBLOCK
>+             * Finally bytecode begins with
>+             * JSOP_SETSP JSOP_EXCEPTION
>+             */
>+            pc = script->main + tn->catchStart;
>+            JS_ASSERT(*pc == JSOP_SETSP);
>+            if (pc[js_CodeSpec[JSOP_SETSP].length] == JSOP_EXCEPTION)
>+                return pc;
>+            JS_ASSERT(pc[js_CodeSpec[JSOP_SETSP].length] == JSOP_ENTERBLOCK);
>+        }
>+    } while ((++tn)->catchStart != 0);

Thanks for following up on python-dev; I wouldn't have thought to bug anyone, since this is a JS problem and not a perceived Python problem.  But it looks like you made some headway.  There is still confusion over what "original problem" might be solved, and whether forcing return via close would solve it.  My understanding is that the original problem from bug 349012, filed by Bob Ippolito as a Python bug, would be solved by changing close to "throw" return.  Is this correct?

/be
(In reply to comment #11)
> There is still confusion over what "original
> problem" might be solved, and whether forcing return via close would solve it. 
> My understanding is that the original problem from bug 349012, filed by Bob
> Ippolito as a Python bug, would be solved by changing close to "throw" return. 
> Is this correct?

The other possible "original problem" is whether yield from close throws, or is detected by close, which then throws per PEP 342.

It's true that this bug's patch or any equivalent for Python, doesn't solve that "problem".  Phillip J. Eby reiterates the Python position that a broken generator that yields from close should not be given the chance to run finally clauses, at http://mail.python.org/pipermail/python-dev/2006-August/068451.html.  It's not a totally convincing argument in light of the fact that the close, if explicit, aborts itself, leaving the generator open -- and the GC will close it again. Wasn't the generator judged broken?  Why should it be run again?

I want to make sure we don't need to revisit this problem here in light of the patch, or in bug 349012 -- or in a new bug (where it arguably belonged all along instead of adding to bug 349012).

/be
(In reply to comment #11)
> My understanding is that the original problem from bug 349012, filed by Bob
> Ippolito as a Python bug, would be solved by changing close to "throw" return. 
> Is this correct?

No, the original problem is that with GeneratorExit it is not possible to write bulletproof exception recovery code since one can not distinguish between legitimate and accidental GeneratorExit:

function gen() {
   for (;;) { 
       try {
            // Some code
            yield 1;
            // More code
        } catch (e) {
            print("Unexpected exception: "+e);
            // do some exception recovery and restart
        }
    }
}

In light of GeneratorExit this is wrong  since the catch would see the exception. One need to write at least:

function gen() {
   for (;;) { 
       try {
            // Some code
            yield 1;
            // More code
        } catch (e) {
            if (e instanceof GeneratorExit)
                throw e;
            print("Unexpected exception: "+e);
            // do some exception recovery and restart
        }
    }
}

Still this version has a deficiency as it does not take into account bugs leading to calling throw GeneratorExit accidentally. Replacing GeneratorExit by the asynchronous return (hence areturnException in the patch) solves this.

In addition it turned out that asynchronous return solve a compatibility problem with Python 2.4. Consider:

def gen():
  try:
    yield 1
    raise NameException
  except:
    print "Exception"

for i in gen():
  print i
  break

While Python 2.4 prints just 1, Python 2.5c1 prints:
1
Exception

With  asynchronous return implemented, then original behavior would be restored. 
(In reply to comment #11)
> (From update of attachment 235232 [details] [diff] [review] [edit])
> This is getting good.  But why rt->areturnException (the a in the name is
> unclear to me too, but I mean why have it at all)?  There are lots of
> crypto-boolean jsvals, a la JSVAL_HOLE.  You might even use that one (but don't
> define it in any public header).

I considered that but decided to use a proper jsval instead to avoid exposing pseudovalues to the debugger and not to worry about stack scanning code that expects normal values AFAICS.
(In reply to comment #14)
> I considered that but decided to use a proper jsval instead to avoid exposing
> pseudovalues to the debugger and not to worry about stack scanning code that
> expects normal values AFAICS.

Pseudo-booleans are scan-safe.  The debugger won't have much easier time seeing an uncatchable empty string.  I still favor an explicitly distinct jsval that doesn't require an allocation.

Thanks for the explanation in comment 13.  With that, and the patch almost ready to check in, I'm warm again on this, and I'll write it up in the ECMA TG1 wiki.

/be
Semantic change for the better, need to get right for js1.7.

/be
Blocks: geniter
Flags: blocking1.8.1?
Posted patch Implementation v3 (obsolete) — Splinter Review
In this version I use "crypto-boolean" JSVAL_ARETURN instead of memory allocated JSString to denote a special catch-invisible exception.
Attachment #235232 - Attachment is obsolete: true
Attachment #235404 - Flags: review?(brendan)
Attachment #235232 - Flags: review?(brendan)
Comment on attachment 235404 [details] [diff] [review]
Implementation v3

Nice!  Nits only:

> struct JSGenerator {
>     JSGenerator         *next;
>     JSObject            *obj;
>     JSGeneratorState    state;
>     JSStackFrame        frame;
>@@ -118,19 +118,26 @@ struct JSGenerator {
>     ((JSGenerator *) ((uint8 *)(fp) - offsetof(JSGenerator, frame)))
> 
> extern JSObject *
> js_NewGenerator(JSContext *cx, JSStackFrame *fp);
> 
> extern JSBool
> js_CloseGeneratorObject(JSContext *cx, JSGenerator *gen);
> 
>+
>+/*
>+ * Special unique value to implement asynchronous return for generator.close.
>+ * Scripts never see it.
>+ */
>+
>+#define JSVAL_ARETURN BOOLEAN_TO_JSVAL(JS_TRUE + 1)

Indent macro definition to start in same column as other justified stuff (members of JSGenerator above, JSClass globals below).

>+
> #endif
> 
> extern JSClass          js_GeneratorClass;
> extern JSClass          js_IteratorClass;
> extern JSClass          js_StopIterationClass;
>-extern JSClass          js_GeneratorExitClass;

[snip]

>+            /*
>+             * Catch bytecode begins with:
>+             * JSOP_SETSP JSOP_ENTERBLOCK
>+             * Finally bytecode begins with
>+             * JSOP_SETSP JSOP_EXCEPTION
>+             */

Might be more readable to put JSOP_* on same line as "begins with:" (and use a : in both sentences, or neither).  Otherwise indent JSOP_ lines a couple of spaces?


>+++ js/src/jsscript.h	2006-08-25 17:19:03.000000000 +0200
>@@ -92,16 +92,27 @@ struct JSScript {
>                     ++tn_;                                                    \
>                 if (tn_->catchStart)                                          \
>                     catchpc_ = (script)->main + tn_->catchStart;              \
>             }                                                                 \
>         }                                                                     \
>         catchpc = catchpc_;                                                   \
>     JS_END_MACRO
> 
>+#if JS_HAS_GENERATORS

No jsconfig.h nested #include, so this may or may not work as hoped.  Elsewhere, we avoid over-nesting headers by not #if'ing function prototypes, since in C extra ones that have no implementation are harmless.

/be
Attachment #235404 - Flags: review?(brendan) → review+
Flags: blocking1.8.1? → blocking1.8.1+
Target Milestone: --- → mozilla1.8.1
Depends on: 349320
Posted patch Implementation v4 (obsolete) — Splinter Review
The changes to address the nits exceeded the limit of mutations to commit without an extra review.
Attachment #235404 - Attachment is obsolete: true
Attachment #235548 - Flags: review?(brendan)
Something is wrong. The test case given bellow even with the patch prints:

Failed!
catch1=false catch2=true catch3=false
finally1=true finally2=true finally3=true
before 9232, after 9232, break 0817b000
before 9232, after 9232, break 0817b000
Failed!
catch1=false catch2=true catch3=false
finally1=true finally2=true finally3=true

The reason is that after the first finally the interpreter when executing JSOP_THROW pops from the stack not JSVAL_ARETURN but rather some junk value. Thus the second catch is triggered as the exception is no longer JSVAL_ARETURN. If the second catch is altered, the bug goes away.


var catch1, catch2, catch3, finally1, finally2, finally3;
var iter;

function gen()
{
	yield 1;
	try {
		try {
			try {
				yield 1;
			} catch (e) {
				catch1 = true;
			} finally {
				finally1 = true;
			}
		} catch (e) {
			catch2 = true;
		} finally {
			finally2 = true;
		}
	} catch (e) {
		catch3 = true;
	} finally {
		finally3 = true;
	}
}

// test explicit close call
catch1 = catch2 = catch3 = finally1 = finally2 = finally3 = false;
iter = gen();
iter.next();
iter.next();
iter.close();

var passed = !catch1 && !catch2 && !catch3 && finally1 && finally2 && finally3;
if (!passed) {
	print("Failed!");
	print("catch1="+catch1+" catch2="+catch2+" catch3="+catch3);
	print("finally1="+finally1+" finally2="+finally2+" finally3="+finally3);
}

// test GC-invoked close
catch1 = catch2 = catch3 = finally1 = finally2 = finally3 = false;
iter = gen();
iter.next();
iter.next();
iter = null;
gc();
gc();

var passed = !catch1 && !catch2 && !catch3 && finally1 && finally2 && finally3;
if (!passed) {
	print("Failed!");
	print("catch1="+catch1+" catch2="+catch2+" catch3="+catch3);
	print("finally1="+finally1+" finally2="+finally2+" finally3="+finally3);
}
(In reply to comment #20)
> Something is wrong.

The patch is innocent, this is already existing problem, see bug 350312.

Depends on: 350312
Comment on attachment 235548 [details] [diff] [review]
Implementation v4

For some reason, quilt produces patches that confuse interdiff.  I used diff instead, and re-read the old patch and then this one.

>+            if (gen->state == JSGEN_OPEN) {
>+                if (js_FindFinallyHandler(gen->frame.script, gen->frame.pc)) {
>+                    /*
>+                     * Generator is yielding inside a try with a finally block.
>+                     * Schedule it for closing.
>+                     */
>+                    *rt->gcCloseState.todoTail = gen;
>+                    rt->gcCloseState.todoTail = &gen->next;
>+                    rt->gcCloseState.todoCount++;
>+                    if (!todo)
>+                        todo = gen;
>+                    METER(JS_ASSERT(rt->gcStats.nclose));
>+                    METER(rt->gcStats.nclose--);
>+                    METER(rt->gcStats.maxcloselater
>+                          = JS_MAX(rt->gcStats.maxcloselater,
>+                                   rt->gcCloseState.todoCount));
>+                } else {
>+#ifdef DEBUG
>+                    gen->state = JSGEN_CLOSED;
>+#endif
>+                }

Why not always set gen->state, not just #ifdef DEBUG?

>+            } else {
>                 /*
>-                 * Generator cannot be nesting, i.e., running or closing, and
>-                 * newborn generator is never registered with GC.
>-                 *
>-                 * XXX: we do need to run the close hook if the last yield
>-                 * happened outside a try block.
>+                 * Unreachable generator registered with reachableList can
>+                 * only be yielding or already closed.

"yielded" or "paused" might be better than yielding, which sounds like it describes the state during JSOP_YIELD, but not after.

>                  */
>-                JS_ASSERT(gen->state == JSGEN_OPEN);
>-                *rt->gcCloseState.todoTail = gen;
>-                rt->gcCloseState.todoTail = &gen->next;
>-                rt->gcCloseState.todoCount++;
>-                if (!todo)
>-                    todo = gen;
>-                METER(JS_ASSERT(rt->gcStats.nclose));
>-                METER(rt->gcStats.nclose--);
>-                METER(rt->gcStats.maxcloselater
>-                      = JS_MAX(rt->gcStats.maxcloselater,
>-                               rt->gcCloseState.todoCount));
>+                JS_ASSERT(gen->state == JSGEN_CLOSED);

Sanity is good even with assertions turned off ;-).

r=me with final nit patrol.

/be
Attachment #235548 - Flags: review?(brendan) → review+
Posted patch Implementation v5 (obsolete) — Splinter Review
I rewrote the comments and removed debug-only gen->state = closed. I hope the comments are clear enough.
Attachment #235548 - Attachment is obsolete: true
Attachment #235584 - Flags: review?(brendan)
To produce patches for attchments I use quilt diff --diff="/usr/bin/diff -U8 -p -d" so the patches are generated by the standard diff. I think it is -p option to the standard diff that confuses interdiff. No quilt to blame!
Comment on attachment 235584 [details] [diff] [review]
Implementation v5

>+++ js/src/jsgc.c	2006-08-26 20:54:16.000000000 +0200
>@@ -928,27 +928,34 @@
> 
>     rt = cx->runtime;
>     todo = NULL;
>     genp = &rt->gcCloseState.reachableList;
>     while ((gen = *genp) != NULL) {
>         if (*js_GetGCThingFlags(gen->obj) & GCF_MARK) {
>             genp = &gen->next;
>         } else {
>+            /*
>+             * Only generators that yilded or were already closed can become

yielded

>+             * unreachable while being registered with reachableList.

or just "while still on reachableList."

/be
Attachment #235584 - Flags: review?(brendan) → review+
Patch to commit with the latest bad-English in comments fixes.
Attachment #235584 - Attachment is obsolete: true
I committed the patch from comment 26 to the trunk:

Checking in js.msg;
/cvsroot/mozilla/js/src/js.msg,v  <--  js.msg
new revision: 3.67; previous revision: 3.66
done
Checking in jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.280; previous revision: 3.279
done
Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.187; previous revision: 3.186
done
Checking in jsgc.c;
/cvsroot/mozilla/js/src/jsgc.c,v  <--  jsgc.c
new revision: 3.172; previous revision: 3.171
done
Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.274; previous revision: 3.273
done
Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v  <--  jsiter.c
new revision: 3.36; previous revision: 3.35
done
Checking in jsiter.h;
/cvsroot/mozilla/js/src/jsiter.h,v  <--  jsiter.h
new revision: 3.13; previous revision: 3.12
done
Checking in jsscript.h;
/cvsroot/mozilla/js/src/jsscript.h,v  <--  jsscript.h
new revision: 3.32; previous revision: 3.31
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I am reopening the bug as in the previous patch I forgot to remove GeneratorExit from jsproto.tbl 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Posted patch jsproto.tbl changes (obsolete) — Splinter Review
Next time I should do grep *, not grep *.c *.h :(
Attachment #235607 - Flags: review?(brendan)
Comment on attachment 235607 [details] [diff] [review]
jsproto.tbl changes

The code values are serialized in the XUL FastLoad file, so let's not change them.  Instead, replace GeneratorExit with UnusedProto28.

An all-in-one patch for 1.8 would be great, for single attachment nomination.

/be
Attachment #235607 - Attachment is obsolete: true
Attachment #235626 - Flags: review?(brendan)
Attachment #235607 - Flags: review?(brendan)
Comment on attachment 235626 [details] [diff] [review]
jsproto.tbl changes v2

Thanks.

/be
Attachment #235626 - Flags: review?(brendan) → review+
I committed the patch from comment 31 to the trunk:

Checking in jsproto.tbl;
/cvsroot/mozilla/js/src/jsproto.tbl,v  <--  jsproto.tbl
new revision: 3.7; previous revision: 3.6
done
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
This is a combined version of patches from comment 26 and comment 31.
Attachment #235645 - Flags: approval1.8.1?
Whiteboard: [baking until 8/29]
Depends on: 350393
No longer depends on: 350393
Comment on attachment 235645 [details] [diff] [review]
Combined patch for 1.8.1 branch

a=schrep/beltnzer for drivers.
Attachment #235645 - Flags: approval1.8.1? → approval1.8.1+
Checking in regress-349331.js;
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-349331.js,v  <--  regress-349331.js
initial revision: 1.1
Flags: in-testsuite+
Unrelated commits to js/src/js.msg required the update of the patch. I ask for aproval once again for the adjusted patch.
Attachment #235645 - Attachment is obsolete: true
Attachment #235909 - Flags: approval1.8.1?
Comment on attachment 235909 [details] [diff] [review]
Combined patch for 1.8.1 branch v2

a=beltzner on behalf of 181drivers
Attachment #235909 - Flags: approval1.8.1? → approval1.8.1+
I committed the patch from comment 37 to MOZILLA_1_8_BRANCH:

Checking in js.msg;
/cvsroot/mozilla/js/src/js.msg,v  <--  js.msg
new revision: 3.43.8.8; previous revision: 3.43.8.7
done
Checking in jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.214.2.26; previous revision: 3.214.2.25
done
Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.128.2.26; previous revision: 3.128.2.25
done
Checking in jsgc.c;
/cvsroot/mozilla/js/src/jsgc.c,v  <--  jsgc.c
new revision: 3.104.2.18; previous revision: 3.104.2.17
done
Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.48; previous revision: 3.181.2.47
done
Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v  <--  jsiter.c
new revision: 3.17.2.12; previous revision: 3.17.2.11
done
Checking in jsiter.h;
/cvsroot/mozilla/js/src/jsiter.h,v  <--  jsiter.h
new revision: 3.6.2.7; previous revision: 3.6.2.6
done
Checking in jsproto.tbl;
/cvsroot/mozilla/js/src/jsproto.tbl,v  <--  jsproto.tbl
new revision: 3.4.2.4; previous revision: 3.4.2.3
done
Checking in jsscript.h;
/cvsroot/mozilla/js/src/jsscript.h,v  <--  jsscript.h
new revision: 3.25.4.3; previous revision: 3.25.4.2
done
Keywords: fixed1.8.1
1.8 20060830 
debug
 Assertion failure: pc[js_CodeSpec[JSOP_SETSP].length] == JSOP_ENTERBLOCK, at /work/mozilla/builds/ff/2.0/mozilla/js/src/jsemit.c:6382

>	js3250.dll!JS_Assert(const char * s=0x101286f0, const char * file=0x101286b8, int ln=6382)  Line 59	C
 	js3250.dll!js_FindFinallyHandler(JSScript * script=0x046fc3e8, unsigned char * pc=0x046fc428)  Line 6382 + 0x2c bytes	C
 	js3250.dll!js_Interpret(JSContext * cx=0x03dca2e0, unsigned char * pc=0x046fc421, long * result=0x0012e964)  Line 6245 + 0x10 bytes	C
 	js3250.dll!SendToGenerator(JSContext * cx=0x03dca2e0, int op=3, JSObject * obj=0x0469e328, JSGenerator * gen=0x03fbc6e0, long arg=-2147483647, long * rval=0x0012eab0)  Line 764 + 0x14 bytes	C
 	js3250.dll!generator_op(JSContext * cx=0x03dca2e0, int op=3, JSObject * obj=0x0469e328, unsigned int argc=0, long * argv=0x047410f4, long * rval=0x0012eab0)  Line 881 + 0x1d bytes	C
 	js3250.dll!generator_close(JSContext * cx=0x03dca2e0, JSObject * obj=0x0469e328, unsigned int argc=0, long * argv=0x047410f4, long * rval=0x0012eab0)  Line 918 + 0x1b bytes	C
 	js3250.dll!js_Invoke(JSContext * cx=0x03dca2e0, unsigned int argc=0, unsigned int flags=0)  Line 1350 + 0x1a bytes	C

opt windows crashed, but not today's nightly.

verified fixed 1.9 20060830 windows/mac*/linux
Status: RESOLVED → VERIFIED
verified fixed 1.8 20060831 windows/mac*/linux. I no longer get the assert.
You need to log in before you can comment on or make changes to this bug.