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

getSourcePositionFromJavaStack cannot perform well on Android system #1680

Closed
anonyein opened this issue Oct 5, 2024 · 9 comments · Fixed by #1761
Closed

getSourcePositionFromJavaStack cannot perform well on Android system #1680

anonyein opened this issue Oct 5, 2024 · 9 comments · Fixed by #1761
Labels
Android Issues related to running Rhino on Android Regression

Comments

@anonyein
Copy link

anonyein commented Oct 5, 2024

f3f1b3f#diff-d0e61fbbe651a547bf81371014f77ac41baa787abac5f56122e9b5e791be91dcR2579
https://github.com/requirejs/r.js

File rJs = new File(rhinoJsDir, "r.js");
if (rJs.exists() && rJs.isFile()) {
    try (FileInputStream inputStream = new FileInputStream(rJs)) {
        InputStreamReader envReader = new InputStreamReader(inputStream);
        cx.evaluateReader(scope, envReader,
                "r.js", 1, null);
    } catch (RhinoException re) {
    }
}

cannot be evaluated after this commit with desugar_jdk_libs on Android system.
Maybe desugar_jdk_libs changes "java.util.Optional" to "$j.util.Optional".
I have reverted this commit.

@p-bakker p-bakker added Android Issues related to running Rhino on Android Regression labels Oct 5, 2024
@p-bakker p-bakker added this to the Release 1.7.16 milestone Oct 5, 2024
@rPraml
Copy link
Contributor

rPraml commented Oct 6, 2024

Sorry for this. I throughd, android uses Interpreter mode only and we should not get there. Can you (or anyone) tell me, how to run tests on android?

@p-bakker
Copy link
Collaborator

p-bakker commented Oct 6, 2024

Just for my info: which Android API version are you're on when running into this compatibility issue? Afaic java.utils.Optional should be supported from v24 onwards

@rbri
Copy link
Collaborator

rbri commented Oct 6, 2024

if i got @anonyein right this is not a runtime issue it happens at build time - @anonyein correct?

@anonyein
Copy link
Author

anonyein commented Oct 6, 2024

@rbri @p-bakker I found this not the problem of Android API version and desugar_jdk_libs. This also happens on Android 11. I will decompile "Context" class.
classes.zip
rhino-all-1.7.16-SNAPSHOT.jar.zip

    static String getSourcePositionFromStack(int[] linep) {
        Evaluator evaluator;
        Context cx = getCurrentContext();
        if (cx == null) {
            return null;
        }
        return (cx.lastInterpreterFrame == null || (evaluator = createInterpreter()) == null) ? getSourcePositionFromJavaStack(linep) : evaluator.getSourcePositionFromStack(cx, linep);
    }

    static String getSourcePositionFromJavaStack(int[] linep) {
        Optional<StackWalker.StackFrame> frame = (Optional) StackWalker.getInstance().walk(new Context$.ExternalSyntheticLambda1());
        return (String) frame.map(new Context$.ExternalSyntheticLambda2(linep)).orElse(null);
    }

    static String lambda$getSourcePositionFromJavaStack$2(int[] linep, StackWalker.StackFrame f) {
        linep[0] = f.getLineNumber();
        return f.getFileName();
    }

    public static boolean frameMatches(StackWalker.StackFrame frame) {
        return (frame.getFileName() == null || !frame.getFileName().endsWith(".java")) && frame.getLineNumber() > 0;
    }
package org.mozilla.javascript;

import java.util.function.Function;
import java.util.stream.Stream;

public final class Context$$ExternalSyntheticLambda1 implements Function {
    @Override
    public final Object apply(Object obj) {
        return Context.lambda$getSourcePositionFromJavaStack$1((Stream) obj);
    }
}
package org.mozilla.javascript;

import java.lang.StackWalker;
import java.util.function.Function;

public final class Context$$ExternalSyntheticLambda2 implements Function {
    public final int[] f$0;

    @Override
    public final Object apply(Object obj) {
        return Context.lambda$getSourcePositionFromJavaStack$2(this.f$0, (StackWalker.StackFrame) obj);
    }
}

@rbri
Copy link
Collaborator

rbri commented Oct 6, 2024

looks like StackWalker was added at API Level 34 / Android 14 (https://developer.android.com/reference/java/lang/StackWalker)

@anonyein
Copy link
Author

anonyein commented Oct 6, 2024

looks like StackWalker was added at API Level 34 / Android 14 (https://developer.android.com/reference/java/lang/StackWalker)

All right, that's too high, and desugar_jdk_libs has not contained this.

@p-bakker
Copy link
Collaborator

p-bakker commented Oct 6, 2024

Can you (or anyone) tell me, how to run tests on android?

This has been a pending question without a cheat answer for quite a while: #1152

@gbrail
Copy link
Collaborator

gbrail commented Dec 18, 2024

It looks like we could address this by replacing the StackWalker with the old "new Throwable().getStackTrace()" technique. That would prevent any Android problems (as shown by the fact that we had to add an annotation here for ErrorProne.

@rbri do you recall why we changed this and whether it'd make sense to change it back?

@rbri
Copy link
Collaborator

rbri commented Dec 18, 2024

my memory says it was done to modernize the code based on jdk11 but i was not the author.
For core-js i had to revert that to be jdk8 and android compatible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to running Rhino on Android Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants