Interpreter.interpretLoop has instructionCount increments without calling Context.observeInstructionCount



11 years ago
11 years ago


(Reporter: Nam.Tuan, Unassigned)






11 years ago
User-Agent:       Mozilla/5.0 (X11; U; SunOS i86pc; en-US; rv: Gecko/20080421 Firefox/
Build Identifier: rhino1_7R1

At we have:

    case Token.NEW : {
        if (instructionCounting) {
            cx.instructionCount += INVOCATION_COST;

Here, the instructionCount increment is without giving context a chance to monitor the execution like in other cases where the increment is done through method Interpreter.addInstructionCount(...).

The same irregularity is also found with instructions REF_CALL and CALLSPECIAL.

I am not sure the skipping of Context.observeInstructionCount() is intentional, but IMO the inconsistence would create blind spots for observer code.

Reproducible: Always

Steps to Reproduce:
This test code would cause rhino execution get out of memory error, without any possible intervention from context observer:

function Foo1() {
    this.s1 =  String("very string #"); 
    this.s10 =  String(s1 + 9);

function Foo2() {
    this.f1 =  new Foo1();
    this.f10 =  new Foo1();

function Foo3() {
    this.f1 =  new Foo2();
    this.f10 =  new Foo2();

function Foo10() {
    this.f1 =  new Foo9();
    this.f10 =  new Foo9();

var huge = new Foo10();

Actual Results:  

Expected Results:  
Context.observeInstructionCount could be used to prevent memory abuser.

At and similar places for REF_CALL and CALLSPECIAL.

    case Token.NEW : {
        if (instructionCounting) {
            addInstructionCount(cx, INVOCATION_COST);

    private static void addInstructionCount(Context cx, int count) {
        cx.instructionCount += count;
        if (cx.instructionCount > cx.instructionThreshold) {
            cx.instructionCount = 0;

Comment 1

11 years ago
I think the current code is correct. The idea is that we only check against thresholds on jumps within frames, and that we then add in all the instructions traversed in that block of code. This way we avoid the performance penalty of checking against the threshold on every instruction. 

Since the various forms of call don't jump *within the frame*, I think all the instructions are properly accounted for. 

The instruction threshold code isn't intended to protect against OutOfMemoryErrors like you show, only against infinite or long-running loops. Your example doesn't actually execute a lot of instructions.
Last Resolved: 11 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.