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

Comment that JVM_DoPrivileged should not be removed #20846

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

theresa-m
Copy link
Contributor

No description provided.

@pshipton
Copy link
Member

You'll need to remove the rest of it, from exports.cmake, j9vmnatives.xml, forwarders.m4

@pshipton
Copy link
Member

Also pls check jdk8 or 11 or both, with an internal build on Windows, since Windows isn't working for PR builds.

@pshipton
Copy link
Member

Actually check jdk8 Windows on vmfarm, I'm not sure if we can remove this.

@theresa-m
Copy link
Contributor Author

theresa-m commented Dec 18, 2024

You're right. It worked with linux 8,11,17,next that I ran previously but caused a Windows vmfarm build to fail compilation. I will add a comment to this method that it shouldn't be removed since its not obvious searching through the OpenJ9 code base.

@theresa-m theresa-m changed the title Remove unused method JVM_DoPrivileged Comment that JVM_DoPrivileged should not be removed Dec 18, 2024
@pshipton
Copy link
Member

pshipton commented Dec 18, 2024

Linux uses dynamic linking and won't fail for a missing symbol until runtime. Windows does static linking and fails during the build. I agree JVM_DoPrivileged is not used/not called, but although we've optimized Semeru to remove the code that calls it, things are different for IBM Java 8.

@@ -380,7 +380,7 @@ java_lang_J9VMInternals_doPrivilegedWithException(JNIEnv* env)
return cached;
}


/* Do not remove: this method is used in Windows Java 8 vmfarm builds. */
Copy link
Member

Choose a reason for hiding this comment

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

Not sure "used" is accurate. It should never be called, but there is code that links to it.

Copy link
Member

@pshipton pshipton Dec 18, 2024

Choose a reason for hiding this comment

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

Pls use "Windows IBM Java 8" or similar. I don't think vmfarm should be used, the build platform may change (perhaps it will be jenkins in the future) but the link issue will persist unless the IBM Java 8 JCL code is modified.

@pshipton pshipton merged commit 8a06d93 into eclipse-openj9:master Dec 19, 2024
2 checks passed
@keithc-ca
Copy link
Contributor

Could we make the definition and exports conditional ((JAVA_SPEC_VERSION == 8) && !defined(OPENJ9_BUILD))?

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

Successfully merging this pull request may close these issues.

3 participants