openj9
openj9 copied to clipboard
eclipse-openj9
Fix compile error and add iconv_init() call
- Revert #https://github.com/eclipse-openj9/openj9/pull/20014
- Fix compile error that origLibpath is not defined on z/OS Java 21
- Call iconv_init() in JNI_CreateJavaVM_impl() before the first use of getenv() on z/OS.
Why should we not also revert #20007? That would avoid the compile error relating to origLibpath and eliminate the need for the new call to iconv_init().
Why should we not also revert https://github.com/eclipse-openj9/openj9/pull/20007?
I think the change in https://github.com/eclipse-openj9/openj9/pull/20007 is still needed (append /lib to LIBPATH) for the cases that are not called from launcher.
For situations where the JVM is not started from a standard launcher, I think adding to LIBPATH as the JVM is started is either irrelevant or too late.
For situations where the JVM is not started from a standard launcher, I think adding to LIBPATH as the JVM is started is either irrelevant or too late.
That means users have to take care of the LIBPATH in these situations ?
Yes, but hopefully, "users" in this case means the authors of non-standard launcher code, not end users of an application.
Yes, but hopefully, "users" in this case means the authors of non-standard launcher code, not end users of an application.
FYI @joransiu
I remembered @joransiu mentioned there are some users on z/OS starting up the JVM via JNI_CreateJavaVM().
If we decide to revert https://github.com/eclipse-openj9/openj9/pull/20007, we may need to notice them that they need to take care of LIBPATH.
Yes, but if those users need shared libraries before starting the JVM, they can't expect the JVM to help (because by definition it hasn't started yet).
but if those users need shared libraries before starting the JVM, they can't expect the JVM to help (because by definition it hasn't started yet).
Yes, I agree. They could see issue caused by LIBPATH before calling into our code. We have no control over that.
I should mention in my above comment I am fine reverting https://github.com/eclipse-openj9/openj9/pull/20007.
FYI @joransiu I remembered @joransiu mentioned there are some users on z/OS starting up the JVM via
JNI_CreateJavaVM(). If we decide to revert #20007, we may need to notice them that they need to take care ofLIBPATH.
Yeah, we hit the problem when using Liberty's launcher, which had not updated LIBPATH with /lib. I think having the JVM ensure /lib is appended if necessary makes sense for such custom launchers.
The question is, does it work? We've seen before that things such as LIBPATH need to be set before the process is started. Setting it in the process after it's started doesn't take effect. The java launcher sets it and then forks the JVM process.
If we have the requirement of /lib being in the LIBPATH on z/OS Java 21, it should be documented.
If we have the requirement of
/libbeing in theLIBPATHon z/OS Java 21, it should be documented.
I don't think it's a requirement for z/OS: the need for the library is a feature of the launcher code on z/OS. Other JNI applications may not need it.
Changed this to revert #https://github.com/eclipse-openj9/openj9/pull/20014 and #https://github.com/eclipse-openj9/openj9/pull/20007
I don't think it's a requirement for z/OS: the need for the library is a feature of the launcher code on z/OS. Other JNI applications may not need it.
An application using java/util/zip/GZip APIs will invoke classlib implementation that calls into system zlib library. How would /lib be handled in that case? I think the JLI launcher issue we handled recently was case where the launcher itself had a dependency on system zlib.
@keithc-ca Any comments on https://github.com/eclipse-openj9/openj9/pull/20070#issuecomment-2318517593 ?
We just tested Liberty launcher, and can confirm that we are able to bring up the app server properly without /lib in the LIBPATH now. The changes from #20007 are needed.
The changes from #20007 are needed.
After an offline discussion with @joransiu, I agree we don't want to revert #20007.
After an offline discussion with @joransiu, I agree we don't want to revert https://github.com/eclipse-openj9/openj9/pull/20007.
I've changed this PR back to the original form: revert https://github.com/eclipse-openj9/openj9/pull/20014 + fix compile error and add a iconv_init() call.