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

EclipseLinkASMClassWriter.getLatestOPCodeVersion was emitting an incorrect warning and picking the oldest Java bytecode version if encountering an unsupported Java version (Bugzilla #579391) #18

Merged
merged 1 commit into from Apr 1, 2022

Conversation

petergeneric
Copy link

Bugzilla: https://bugs.eclipse.org/bugs/show_bug.cgi?id=579391

SCENARIO:

Using Eclipselink MOXy (using latest eclipselink-asm) on Java 18

ACTUAL RESULT:

When using EclipseLink MOXy 2.7.8 or 2.7.10 on Java 18, the following warning is printed to stderr:

org.eclipse.persistence.internal.libraries.asm.EclipseLinkASMClassWriter getLatestOPCodeVersion
WARNING: Java SE '18' is too old.

EXPECTED VALUE:

It should be printing:

WARNING: Java SE '18' is not fully supported yet. Report this error to the EclipseLink open source project.

CONSEQUENCE:

This bug also causes EclipseLink ASM to set version to 1.7 (so presumably will generate Java 1.7 bytecode rather than Java 17 bytecode)

The code is using a TreeMap for "versionMap" but then using "lastKey()" as if TreeMap preserved insertion order (or used numerical order), rather than natural string ordering, and lastKey returns "9" since the String "9" > "17"

DESCRIPTION OF CHANGE:

The fix for this is to switching from TreeMap to LinkedHashMap, which preserves insertion order.

In addition to this bug, the Map construction code is pointlessly creating an anonymous class and storing the Map in the heap forever, whereas "versionMap" could be constructed within getLatestOPCodeVersion and populated with calls to .put, removing the need for that anonymous class (and saving wasted heap space as an added benefit)

Additionally, "version" could be made static so that this computation only happens once per class load (and to reduce the number of times the warning might be printed to stderr)

…Version was emitting an incorrect warning and picking the oldest Java bytecode version if encountering an unsupported Java version
Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@lukasj lukasj merged commit 8c819b2 into eclipse-ee4j:master Apr 1, 2022
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.

None yet

2 participants