Skip to content

Fix Java 11 Test Errors #6

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.

Sign up for GitHub

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

Jump to bottom
Merged
ctrueden merged 7 commits into from
Apr 2, 2021
Merged

Fix Java 11 Test Errors #6

ctrueden merged 7 commits into from
Apr 2, 2021

Conversation

@hanslovsky
Copy link
Member

@hanslovsky hanslovsky commented Mar 14, 2021 •
edited
Loading

Summary

Currently, the tests fail for Java 11, and so does running Kotlin Script language from another project that runs on Java 11. Test failure example:

$ JAVA_HOME=/usr/lib/jvm/java-11-openjdk mvn clean package test -Denforcer.skip=true
...
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.scijava.plugins.scripting.kotlin.KotlinTest
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.233 s <<< FAILURE! - in org.scijava.plugins.scripting.kotlin.KotlinTest
[ERROR] org.scijava.plugins.scripting.kotlin.KotlinTest  Time elapsed: 2.23 s  <<< ERROR!
java.lang.StackOverflowError

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   KotlinTest.org.scijava.plugins.scripting.kotlin.KotlinTest » StackOverflow
[INFO] 
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0

Upon some inspection with this @ClassRule:

class LogExceptionRule : TestRule {
    override fun apply(s: Statement?, d: Description?): Statement {
        return object: Statement() {
            override fun evaluate() {
                try {
                    s!!.evaluate()
                } catch (t: Throwable) {
                    System.err.println("Caught throwable ${t.javaClass.simpleName}: ${t.message}")
                    t.printStackTrace()
                }
            }

        }
    }
}

I was able to get the entire stack trace:

Caught throwable StackOverflowError: null
java.lang.StackOverflowError
        at java.base/jdk.internal.reflect.Reflection.verifyMemberAccess(Reflection.java:132)
        at java.base/java.util.ServiceLoader.checkCaller(ServiceLoader.java:568)
        at java.base/java.util.ServiceLoader.<init>(ServiceLoader.java:504)
        at java.base/java.util.ServiceLoader.load(ServiceLoader.java:1647)
        at java.scripting/javax.script.ScriptEngineManager.getServiceLoader(ScriptEngineManager.java:92)
        at java.scripting/javax.script.ScriptEngineManager$1.run(ScriptEngineManager.java:105)
        at java.scripting/javax.script.ScriptEngineManager$1.run(ScriptEngineManager.java:102)
        at java.base/java.security.AccessController.doPrivileged(Native Method)
        at java.scripting/javax.script.ScriptEngineManager.initEngines(ScriptEngineManager.java:101)
        at java.scripting/javax.script.ScriptEngineManager.init(ScriptEngineManager.java:87)
        at java.scripting/javax.script.ScriptEngineManager.<init>(ScriptEngineManager.java:62)
        at org.scijava.plugins.scripting.kotlin.KotlinScriptLanguage$Factory.getScriptEngine(KotlinScriptLanguage.kt:56)
        at org.scijava.script.AdaptedScriptLanguage.getScriptEngine(AdaptedScriptLanguage.java:137)
        at java.scripting/javax.script.ScriptEngineManager.getEngineByExtension(ScriptEngineManager.java:291)
        at org.scijava.plugins.scripting.kotlin.KotlinScriptLanguage$Factory.getScriptEngine(KotlinScriptLanguage.kt:56)
        at org.scijava.script.AdaptedScriptLanguage.getScriptEngine(AdaptedScriptLanguage.java:137)
        at java.scripting/javax.script.ScriptEngineManager.getEngineByExtension(ScriptEngineManager.java:291)
        at ...

KotlinScriptLanguage$Factory.getScriptEngine is called recursively through ScriptEngineManager.getEngineByExtension. It is unclear to me, why this happens (or why it worked at all before with Java 8). A change from Java 8 to Java 11 how javax.script.ScriptEngineFactorys are detected may be a plausible explanation.

This PR calls

KotlinJsr223DefaultScriptEngineFactory().scriptEngine

directly instead of going through ScriptEngineManager.getEngineByExtension.

Fixes #5 if upstream scijava-maven-plugin is updated to include notOptionalFilter in PopulateAppMojo

@hanslovsky hanslovsky marked this pull request as ready for review March 15, 2021 01:59
@hanslovsky hanslovsky changed the title Try fix java11 Fix Java 11 Test Errors Mar 15, 2021
@hanslovsky hanslovsky mentioned this pull request Mar 20, 2021
Closed
@hanslovsky
Copy link
Member Author

hanslovsky commented Mar 21, 2021

I added a profile that is activated by the scijava.app.directory property and creates a shaded Jar that includes the entire kotlin-compiler-embeddable dependencay with relocations of the clashing trove4j dependency. Additionally, the kotlin-compiler-embeddable dependency is marked as optional in this profile to exclude it from installation in to scijava.app.directory. For the latter to work, the PopulateAppMojo would need to add a

TransformableFilter notOptionalFilter = new AbstractFilter() {
	@Override
	public boolean accept(Node node, List<Node> parents) {
		return !node.getDependency().isOptional();
	}
};

similar to InstallArtifactMojo

@hanslovsky hanslovsky mentioned this pull request Mar 22, 2021
Merged
@hanslovsky
Copy link
Member Author

hanslovsky commented Mar 22, 2021 •
edited
Loading

I added an option to ignore optional dependencies with the scijava.ignoreOptionalDependencies option upstream in scijava/scijava-maven-plugin#28

@hanslovsky
Copy link
Member Author

hanslovsky commented Apr 2, 2021

scijava-maven-plugin-2.1.0 was released and this is now ready for another round of reviews.

@ctrueden
Copy link
Member

ctrueden commented Apr 2, 2021

Thanks a lot, @hanslovsky!

@ctrueden ctrueden merged commit 8329a7c into master Apr 2, 2021
@ctrueden ctrueden deleted the try-fix-java11 branch April 2, 2021 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Trove dependency

2 participants

@hanslovsky @ctrueden