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

Tests graalvm-community-jdk21u-issues-28 #306

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

Karm
Copy link
Owner

@Karm Karm commented Dec 17, 2024

Embeds a variation on the reproducer @zakkak outlined on the original issue.
This forces the native agent to register e.g. the getNextThreadIdOffset that later causes

/bin/ld: jdkreflections.o:(.data+0x7b8): undefined reference to `Java_java_lang_Thread_getNextThreadIdOffset'

Fails with current Mandrel for JDK 21, passes with e.g. Mandrel for JDK 23.

 mvn clean verify -Dquarkus.version=3.15.1 -Ptestsuite -Dtest=AppReproducersTest#jdkReflectionsTest

or in a container:

mvn clean verify -Ptestsuite-builder-image 
    -Dtest=AppReproducersTest#jdkReflectionsContainerTest -Dquarkus.version=3.15.1
    -Dquarkus.native.builder-image=quay.io/quarkus/ubi-quarkus-mandrel-builder-image:jdk-23
    -Dquarkus.native.container-runtime=podman
    -Drootless.container-runtime=true -Dpodman.with.sudo=false | tee log.log

...swap jdk-23 with jdk-21 to see the fail.

@Karm
Copy link
Owner Author

Karm commented Dec 18, 2024

Hmm, it worries me that all tests in CI passes. JDK 21 should have failed. Lemme take a look...

@zakkak
Copy link
Collaborator

zakkak commented Dec 18, 2024

Hmm, it worries me that all tests in CI passes. JDK 21 should have failed. Lemme take a look...

You set minJDK = "21.0.3" but the CI uses

  • Mandrel-23.1.1.0-Final which is Java version: 21.0.1+12-LTS
  • Mandrel-23.1.2.0-Final which is Java version: 21.0.2+13-LTS
  • Mandrel-24.0.1.0-Final which is Java version: 22.0.1+8 (which includes the fix)

I suggest bumping both the minJDK and the versions used in the pipelines

Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

LGTM, other than the minJDK version being 21.0.3 while the test is expected to fail with any JDK 21 build.

Copy link
Collaborator

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

OK (with the suggestions from @zakkak)

@Karm Karm requested a review from zakkak December 19, 2024 08:30
@zakkak zakkak merged commit 0e281ea into master Dec 19, 2024
38 of 54 checks passed
@zakkak zakkak deleted the graalvm-community-jdk21u-issues-28 branch December 19, 2024 09:48
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.

3 participants