Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing copy to fix ArrayIndexOutOfBoundException when invoke captureContinuation #1815

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Rhython
Copy link

@Rhython Rhython commented Feb 2, 2025

In the captureContinuation method, stack and stackAttribute are considered to be the same length. The ensureStackLength method does not copy stackAttributes with the new length, so calling captureContinuation after this will throw an ArrayIndexOutOfBoundException. I updated my project to use v1.8.0 and found this issue so I fixed it and create this pr.

@gbrail
Copy link
Collaborator

gbrail commented Feb 3, 2025

Is there a way that you can construct a test case for this? It's important that we have some way of tracking down possible regressions.

@Rhython
Copy link
Author

Rhython commented Feb 4, 2025

Of course, I made a simplified reproduction code, but I'm not sure how to incorporate it into the unit test of the project.

public class TestRhino2 {

    private Scriptable scope;
    private Object continuation;

    public void await() {
        try (Context cx = Context.enter()) {
            throw cx.captureContinuation();
        }
    }

    public void execute(String source) {
        try (Context cx = Context.enter()) {
            scope = cx.initSafeStandardObjects();
            scope.put("t", scope, this);
            cx.executeScriptWithContinuations(cx.compileString(source, "test", 1, null), scope);
        } catch (ContinuationPending e) {
            continuation = e.getContinuation();
        }
    }

    public void resume(Object obj) {
        try (Context cx = Context.enter()) {
            cx.resumeContinuation(continuation, scope, obj);
        } catch (ContinuationPending e) {
            continuation = e.getContinuation();
        }
    }

    public static void main(String[] args) {
        TestRhino2 test = new TestRhino2();

        String js1 = """
                let mainMenu = [];
                let bonusMenu = [];
                // add enough items to inc stack size
                for (let i = 0; i < 30; i++) {
                    mainMenu.push(['MainMenu' + i, 'MainAction' + i]);
                    bonusMenu.push(['BonusMenu' + i, 'BonusAction' + i]);
                }
                Array.prototype.push.apply(mainMenu, bonusMenu) // this will cause ensureStackLength to be called
                let select = t.await() // this will cause ArrayIndexOutOfBoundsException
                """;
        test.execute(js1);
        test.resume(0);

    }

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants