eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.27k stars 721 forks source link

Sort out the extension for AIX on JDK15 in `System.mapLibraryName(userLibName)` #9788

Open DanHeidinga opened 4 years ago

DanHeidinga commented 4 years ago

It would be good to confirm in case we decide to go with this option.

For existing OpenJ9 System.mapLibraryName(userLibName) returning .so, I tested ClassLoaderHelper.mapAlternativeName() patch as following at https://github.com/ibmruntimes/openj9-openjdk-jdk/pull/204:

  1. rename libnio.so to libnio.a;
  2. -version failed with UnsatisfiedLinkError;
  3. -version succeeds with the patch ClassLoaderHelper.mapAlternativeName() which returns .a.

Building a JDK15 AIX to verify that the similar patch (returning .so) also works when System.mapLibraryName(userLibName) returning .a.

Update: Verified that JDK15 AIX still builds with System.mapLibraryName(userLibName) returning .a like JDK 8 & 11, and adding aix/classes/jdk/internal/loader/ClassLoaderHelper.java in which mapAlternativeName() returns .so.

Originally posted by @JasonFengJ9 in https://github.com/eclipse/openj9/pull/9632#issuecomment-631710003

DanHeidinga commented 4 years ago

@JasonFengJ9 Can you comment on this so I can assign it to you? It's the follow on item for figuring out the sharedlib extension for AIX in jdk15

JasonFengJ9 commented 4 years ago

@DanHeidinga yes, I will follow up this issue.

DanHeidinga commented 4 years ago

@andrew-m-leonard Did this get raised with OpenJDK? I'm not sure I saw the outcome from that discussion

JasonFengJ9 commented 4 years ago

@DanHeidinga I believe there were two posts, one is in core-libs-dev and the other is in ppc-aix-port-dev but unfortunately there was no discussion happening.

https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067318.html https://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2020-June/004109.html

DanHeidinga commented 4 years ago

@andrew-m-leonard Can you prod this discussion along at OpenJDK? I believe @JasonFengJ9 has put together a patch that can be added to the discussion

JasonFengJ9 commented 4 years ago

The patch mentioned by Dan is https://github.com/ibmruntimes/openj9-openjdk-jdk/pull/204.

Note: this patch adds jdk.internal.loader. ClassLoaderHelper.mapAlternativeName() to return .a when existing java.lang.System.mapLibraryName(name) returning .so. Hence both .a and .so libraries can be accepted when System.mapLibraryName(name) result doesn't work and ClassLoaderHelper.mapAlternativeName(libname) serves as a fallback to get another lib extension. This patch can switch to return .so in ClassLoaderHelper.mapAlternativeName(libname) instead if OpenJDK community agrees to modify System.mapLibraryName(name) and let it return .a.

andrew-m-leonard commented 4 years ago

@DanHeidinga I will give another prod...!

andrew-m-leonard commented 4 years ago

Matthias has replied: https://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2020-July/004114.html

JasonFengJ9 commented 4 years ago

So I would probably keep it as it is , because we had not really much complains so far about it .

Note that JDK 15+ native library loading doesn't support .a, we might contribute ibmruntimes/openj9-openjdk-jdk#204 to OpenJDK community.

DanHeidinga commented 4 years ago

Upstreaming ibmruntimes/openj9-openjdk-jdk#204 makes sense to me provided we sufficiently motivate for the broader OpenJDK community why the change is needed.

Does the RI's build include .a files - like the libnio.a mentioned in ibmruntimes/openj9-openjdk-jdk#204? Does the RI experience the same issue we had with native library support in 15?

Is there anything we can use to demonstrate that Java applications are broken on AIX without this change?

JasonFengJ9 commented 4 years ago

Does the RI's build include .a files - like the libnio.a mentioned in ibmruntimes/openj9-openjdk-jdk#204? Does the RI experience the same issue we had with native library support in 15?

@DanHeidinga RI build doesn't include .a files, and neither does OpenJ9. libnio.a was renamed from libnio.so manually to demonstrate that the patch with ClassLoaderHelper.mapAlternativeName() can work with both .a and .so suffixes. The native library loading issue OpenJ9 JDK15 had was because of two causes:

  1. Initially OpenJ9 System.mapLibraryName(name) returns .a suffix (current OpenJ9 8/11/14 still return .a suffix);
  2. There is no ClassLoaderHelper. mapAlternativeName() to returns .so suffix when the native library loading failed with .a suffix https://github.com/ibmruntimes/openj9-openjdk-jdk15/blob/d8ad2be09f3917e6b6587caf0c80ad4e7487e46d/src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java#L307-L323.

We introduced a PR to return .so suffix for OpenJ9 JDK 15+ System.mapLibraryName(name), i.e., fixed the cause 1 above, it is sufficient to allow OpenJ9 JDK 15+ build without addressing the cause 2.

The patch ibmruntimes/openj9-openjdk-jdk#204 serves an insurance to support .a suffix library since both formats are supported but it is not required for current JDK 15+ build.

Is there anything we can use to demonstrate that Java applications are broken on AIX without this change?

Not for JDK15+, as mentioned above, System.mapLibraryName(name) returning .so fixed the build breakage, and currently there is no .a libraries in JDK itself.

JasonFengJ9 commented 4 years ago

Is there anything we can use to demonstrate that Java applications are broken on AIX without this change?

There was an issue in WAS though: https://www.ibm.com/support/knowledgecenter/SSEQTP_8.5.5/com.ibm.websphere.base.doc/ae/rtrb_classload_viewer.html#rtrb_classload_viewer__sysmap

System.mapLibraryName returns the wrong library file. When loading a shared library, JVM calls mapLibraryName(libName) to convert libName to a platform specific name. On AIX®, HP-UX or Solaris operating systems, this call might return a file name with the wrong extension (for example, libName.so rather than libName.a). To debug this, write a program to that calls System.mapLibraryName() and verify that it returns the correct file name.

DanHeidinga commented 4 years ago

We're really talking about JNI natives here. Is there a popular project that builds .a files rather than .so files on AIX?

JasonFengJ9 commented 4 years ago

Is there a popular project that builds .a files rather than .so files on AIX?

@zl-wang any insights?

andrew-m-leonard commented 4 years ago

If it seems 50:50, is there no real right answer...? so what Matthias said is probably what the community will go with, ie.keep it as it is...

zl-wang commented 4 years ago

as far as i can see, the OSS community on AIX generally builds .a version, and sometimes builds both .a and .so (to be downloaded for /opt/freeware).

JasonFengJ9 commented 4 years ago

the OSS community on AIX generally builds .a version

Just wondering if there is any JNI native falling into this category? specifically, does AIX OSS community build/distribute any JNI native primarily in .a format? If so, it probably is going to have problem to be loaded by OpenJ9 JDK15+ via System.mapLibraryName(name) (Assuming it works with earlier IBM JDK versions).

zl-wang commented 4 years ago

Just wondering if there is any JNI native falling into this category? specifically, does AIX OSS community build/distribute any JNI native primarily in .a format?

I am not aware, but I cannot exclude the possibility.

DanHeidinga commented 4 years ago

Moving this out based on community call discussion indicating that this isn't required but should be documented in the release notes for 0.22

pshipton commented 3 years ago

The change has been documented, moving this to the backlog.

kohlschuetter commented 7 months ago

This change in behavior broke loading of the native library for junixsocket on JDK15 and newer. We will provide a fix on our end to work around the regression.

It should be noted that this is not only an AIX problem, but also one on IBM i, which previously returned yet another library suffix, .srvpgm.

The problem arises when using System.mapLibraryName in combination with System.load (instead of System.loadLibrary).

Our workaround is basically as follows:

    String mappedName = System.mapLibraryName(libraryNameAndVersion);
    if (mappedName.endsWith(".so")) {
      switch (System.getProperty("os.name", "")) {
        case "AIX":
          mappedName = mappedName.substring(0, mappedName.length() - 3) + ".a";
          break;
        case "OS/400":
          mappedName = mappedName.substring(0, mappedName.length() - 3) + ".srvpgm";
          break;
        default:
          break;
      }
    }
    System.load(absolutePathToLibraries + "/" + mappedName);
JasonFengJ9 commented 7 months ago

It should be noted that this is not only an AIX problem, but also one on IBM i, which previously returned yet another library suffix, .srvpgm.

https://github.com/eclipse-openj9/openj9/blob/8dc58cc374514bbd4864e7194b2a4587bf3f9d57/runtime/jcl/unix/syshelp.c#L218-L231 The IBM i still returns .srvpgm though, there was no change on that platform.

kohlschuetter commented 7 months ago

@JasonFengJ9 The results I'm seeing beg to differ.

public class MapTest {
  public static void main(String[] args) {
    System.out.println(System.mapLibraryName("test"));
  }
}
-bash-5.2$ export JAVA_HOME=/QOpenSys/QIBM/ProdData/JavaVM/jdk11/64bit
-bash-5.2$ export PATH=$JAVA_HOME/bin:$PATH
-bash-5.2$ java -cp . MapTest
test.srvpgm
-bash-5.2$ export JAVA_HOME=/QOpenSys/QIBM/ProdData/JavaVM/jdk17/64bit
-bash-5.2$ export PATH=$JAVA_HOME/bin:$PATH
-bash-5.2$ java -cp . MapTest
libtest.so

Notably, this is being invoked from a PASE environment, obviously. So there may be yet another factor at play when PASE is not involved.

By the way, I'm also not arguing that .srvpgm would be the correct suffix here — in PASE on IBM i, I only see .a and .so for libraries. It's just that this Java API erroneously assumes that there is only one canonical suffix, yet 1. there is obviously more than one, and 2. that this behavior has changed from one Java version to another.

JasonFengJ9 commented 7 months ago

@kohlschuetter I agree your test result seems odd, just wondering if J9OS_I5 was enabled for the JDK17 in question, could you run /QOpenSys/QIBM/ProdData/JavaVM/jdk17/64bit/bin/java -XshowSettings:properties and provide the system property values for os.name, java.vm.info, and java.fullversion?

It's just that this Java API erroneously assumes that there is only one canonical suffix, yet 1. there is obviously more than one, and 2. that this behavior has changed from one Java version to another.

AIX has a unique suffix scenario, both .a and .so libraries are supported and can be loaded. When a platform-independent library name is supplied such as System.loadLibrary(), lib*.so will be attempted first, if the loading fails, lib*.a is to be tried as well. On the other hand System.mapLibraryName() can only return one platform-specific file name. The behaviour change was documented at

This matches HotSpot behaviour which is the reference implementation. The workaround in https://github.com/eclipse-openj9/openj9/issues/9788#issuecomment-1925301622 will be required for JDK15+ if Sysmtem.load() with an absolute path name is used.

kohlschuetter commented 7 months ago

@JasonFengJ9

Here's the output from /QOpenSys/QIBM/ProdData/JavaVM/jdk17/64bit/bin/java -XshowSettings:properties

    java.vm.info = JRE 17 OS/400 ppc64-64-Bit Compressed References 20231209_000000 (JIT enabled, AOT enabled)
OpenJ9   - 461bf3c70
OMR      - 5eee6ad9d
JCL      - 356287fbde7 based on jdk-17.0.9+9
    os.name = OS/400
    java.fullversion = 17.0.9+9
JRE 17 OS/400 ppc64-64-Bit Compressed References 20231209_000000 (JIT enabled, AOT enabled)
OpenJ9   - 461bf3c70
OMR      - 5eee6ad9d
JCL      - 356287fbde7 based on jdk-17.0.9+9
JasonFengJ9 commented 7 months ago

Thanks @kohlschuetter , OS/400 appeared and it seems J9OS_I5 was enabled.

There are no trace points in syshelp.c:mapLibraryToPlatformName() or system.c:Java_java_lang_System_mapLibraryName(), the method tracing for System.mapLibraryName() probably won't show much useful info. I think some IBM i team help is needed. @pshipton do you have any contacts?

pshipton commented 7 months ago

I don't see that IBM i participate in OpenJ9, and Openj9 doesn't support IBM i, any issue should be taken up via IBM support.

pshipton commented 7 months ago

What OpenJDK does is try mapLibraryName() and then ClassLoaderHelper.mapAlternativeName(), but since ClassLoaderHelper isn't public, this isn't helpful outside the JCL itself.

https://github.com/ibmruntimes/openj9-openjdk-jdk17/blob/openj9/src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java#L326 https://github.com/ibmruntimes/openj9-openjdk-jdk17/blob/openj9/src/java.base/aix/classes/jdk/internal/loader/ClassLoaderHelper.java#L55

kohlschuetter commented 7 months ago

@pshipton I don't have an IBM support contract for IBM i; I'm seeing this on pub400.com. My educated guess is that IBM i builds an AIX-like config for the PASE subsystem. I've found a workaround for my library, so it's no pressing issue. However, it would be great if some of your IBM i colleagues could chime in here or reach out to me directly (besides this issue: I think I've uncovered a bug on IBM i with regards to Unix domain sockets, alas).

pshipton commented 7 months ago

The forum for pub400.com isn't helpful?

kohlschuetter commented 7 months ago

@pshipton I thought the OpenJ9 issue tracker would be the best place to raise OpenJ9 issues.

Again: I found a workaround. I only wanted to tell you that there's a problem with IBM i, which indeed runs a version of OpenJ9, that, as it sounds, is doing things in a way you might've not been be aware of. So what's left is an exercise for someone at 👁️ 🐝 Ⓜ️ to call someone else at 👁️ 🐝 Ⓜ️, or not :)

ThePrez commented 7 months ago

@kohlschuetter thanks for reporting this.

It is my belief that the JV1 version of Java on IBM i should indeed return .srvpgm as has been done in the past. However, I've done some code analysis and see that removal of the .srvpgm logic was explicit. We'll look into it.