Include classloader name and module version in stack trace

[sharon-wang]

Fixes: #11452
Fixes: #11447

Per the javadoc, if the classloader is named and it is not one of the built-in classloaders: Bootstrap, Platform or System (aka App), then it should be included when printing StackTraceElement.

Javadoc:

If a class is defined in an unnamed module then the second element is omitted as shown in "com.foo.loader//com.foo.bar.App.run(App.java:12)".

If the class loader is a built-in class loader or is not named then the first element and its following "/" are omitted as shown in "acme@2.1/org.acme.Lib.test(Lib.java:80)". If the first element is omitted and the module is an unnamed module, the second element and its following "/" are also omitted as shown in "MyClass.mash(MyClass.java:9)".

The reference implementation appears to show app/<MODULE>/ in some cases (eg. #11452 (comment)), which is the System/App classloader, a built-in classloader. Although this appears to contradict the javadoc, it seems to only apply when a stack trace is printed by a class other than Throwable or StackFrame, such as ThreadInfo.

This PR matches the RI in displaying the classloader name if the element's classloader is the System/App classloader for classes other than Throwable or StackFrame, since it's the expected output of the stack trace for users. So, we will only omit the classloader name if the classloader is one of the Built-in classloaders and the calling class is Throwable or StackFrame.

Additionally, the RI shows the module version for StackTraceElement, Throwable, StackFrame and ThreadInfo by default for classes loaded by the App classloader, other than system module classes. For ThreadInfo or StackTraceElements instantiated using a public constructor, the module version is always printed, even for modules such as java.base. This PR also matches this behaviour.

Note: StackFrame output was not fixed in the PR and will be addressed in a future PR. Opened: #11774

[pshipton]

How does the stack trace with module version printed compare with the RI? I think it should also be printed only when !classLoaderIsBuiltIn.

[pshipton]

Re showing app, it's better to match the RI than to match the javadoc.
a) user's expect the behavior of the RI
b) historically when the javadoc doesn't match the implementation and this is pointed out, most frequently the javadoc is updated.

[sharon-wang]

How does the stack trace with module version printed compare with the RI? I think it should also be printed only when !classLoaderIsBuiltIn.

Yes, in the RI the module version is printed regardless of if classLoaderIsBuiltIn. My changes match this behaviour: as long as there is a module name, if there's also a module version then the module version will be printed as well.

[sharon-wang]

Re showing app, it's better to match the RI than to match the javadoc.
a) user's expect the behavior of the RI
b) historically when the javadoc doesn't match the implementation and this is pointed out, most frequently the javadoc is updated.

Sounds good. We can include the classloader name:

if ((!classLoaderIsBuiltIn) || (classLoaderName.equals("app")))

Or, I can modify the calculation for classLoaderIsBuiltIn to only care about the Bootstrap and Extension (platform) classloaders.

[sharon-wang]

I've updated the PR title and description to match the changes. StackTraceElement, Throwable and ThreadInfo now all do the same thing, with Throwable and ThreadInfo using StackTraceElement.toString() to set the string for each StackTraceElement.

Module versions are included by default and classloader names are printed unless the classloader is the Bootstrap or Platform (Extension) classloader.

[pshipton]

Yes, in the RI the module version is printed regardless of if classLoaderIsBuiltIn.

This doesn't make sense to me. i.e. normal stack traces don't show module versions for java.base, yet the StackTraceElement contains a module name and version.

For example

import java.util.*;

public class Test1 {
public static void main(String[] args ) throws Throwable {
        System.out.println(String.class.getModule().getDescriptor().version());
        Vector v = new Vector();
        try {
                v.firstElement();
        } catch (Throwable e) {
                e.printStackTrace();
                StackTraceElement ste = e.getStackTrace()[0];
                System.out.println(ste.getModuleName());
                System.out.println(ste.getModuleVersion());
        }
}
}

I happened to run this with Java 16, but same with Java 11, just substitute (for example) "11.0.9".

Optional[16]
java.util.NoSuchElementException
	at java.base/java.util.Vector.firstElement(Unknown Source)
	at Test1.main(Test1.java:8)
java.base
16

[sharon-wang]

Yes, in the RI the module version is printed regardless of if classLoaderIsBuiltIn.

This doesn't make sense to me. i.e. normal stack traces don't show module versions for java.base, yet the StackTraceElement contains a module name and version.

Did some more testing. The behaviour is different between Throwable and ThreadInfo.

For Throwable, I'm getting the same output shown in #11601 (comment). I'd also like to note that app// doesn't show up in the output.

However, for ThreadInfo, for example

import java.lang.management.*;

public class Main {
	public static void main(String[] args) throws Exception {
		ThreadMXBean tb = ManagementFactory.getThreadMXBean();
		ThreadInfo[] tis = tb.dumpAllThreads(true, true);
		for (ThreadInfo ti : tis) {
				System.out.println(ti);
		}
	}
}

the RI shows:

jdk-11.0.9.1+1/Contents/Home/bin/java Main                            
"main" prio=5 Id=1 RUNNABLE
	at java.management@11.0.9.1/sun.management.ThreadImpl.dumpThreads0(Native Method)
	at java.management@11.0.9.1/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:521)
	at java.management@11.0.9.1/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:509)
	at app//Main.main(Main.java:43)

"Reference Handler" daemon prio=10 Id=2 RUNNABLE
	at java.base@11.0.9.1/java.lang.ref.Reference.waitForReferencePendingList(Native Method)
	at java.base@11.0.9.1/java.lang.ref.Reference.processPendingReferences(Reference.java:241)
	at java.base@11.0.9.1/java.lang.ref.Reference$ReferenceHandler.run(Reference.java:213)
...

So, for Throwable, we don't want to show app nor the module version and for ThreadInfo we want to show both.

[Thihup]

Can you take a look at #11523?
Looks like it may help in something

[pshipton]

Note StackFrameImpl.toStackTraceElement() in StackWalker.java should set any extra fields added in StackTraceElement. It's going to be used in #11631

[sharon-wang]

Took some time to think about the design a bit more.

There are two main settings we're concerned about: includeModuleVersion and includeClassLoaderName.

For Throwable or StackFrame:

• includeModuleVersion = true if the declaring class is not one of the JDK classes, such as those in java.base or java.management; false otherwise.
  • If the classloader is the Bootstrap or Platform/Extension classloader, then I think that should cover any of the provided JDK classes? So, set includeModuleVersion = false if the classloader is the Bootstrap or Platform/Extension classloader?
• includeClassLoaderName = true if the classloader is not the Bootstrap, Platform/Extension or System/App classloader (not a built-in classloader); false otherwise.

For ThreadInfo or other instances of StackTraceElement created from its public constructors:

• includeModuleVersion = true by default (always include module version).
• includeClassLoaderName = true if the classloader is not the Bootstrap or Platform/Extension classloader; false otherwise.

Currently, I natively calculate which classloader is being used in jclexception.c, but this isn't useful when public constructors for StackTraceElement are used, which is happening with ThreadInfo and StackFrame. So, this calculation may be better placed in jcl/src/java.base/share/classes/com/ibm/oti/util/Util.java, leveraging VMAccess.java to retrieve classloaders and compare them. However, I don't have access to the declaring class, except in StackFrame, which I would need to grab the classloader of the declaring class to check against the built-in classloaders.

So, I think the main challenge right now is getting access to the declaring class, and then accessing the declaring class's classloader.

[sharon-wang]

Opened Fix StackFrame stack trace output #11774

[sharon-wang]

@pshipton Added/replaced relevant Sidecar19 with [IF JAVA_SPEC_VERSION >= 11]; left jclexception as-is without ifdef

[pshipton]

jenkins compile win jdk8

[pshipton]

jenkins test sanity.openjdk zlinux jdk11

[pshipton]

18:32:45  common/jclexception.c(407) : error C2220: warning treated as error - no 'object' file generated
18:32:45  common/jclexception.c(407) : warning C4244: '=' : conversion from 'UDATA' to 'U_32', possible loss of data
18:32:45  common/jclexception.c(408) : warning C4244: '=' : conversion from 'UDATA' to 'U_32', possible loss of data

[sharon-wang]

Build_JDK11_s390x_linux_Personal, Test_openjdk11_j9_sanity.openjdk_s390x_linux_Personal passed.

For the JDK 8 failure, I added a JAVA_SPEC_VERSION >= 11 check around introduced code in jclexception.c in this last push and it's getting past the failure point in personal testing on JDK 8.

[pshipton]

Not sure the #if is going to fix Windows. I think you need a cast because on Windows BOOLEAN is UDATA while on other platforms it's U_32.

jenkins compile win jdk8,jdk11

[pshipton]

Still failing on Windows in the jdk11 build.

[2021-01-26T17:05:06.306Z] F:\Users\jenkins\workspace\Build_JDK11_x86-64_windows_Personal\openj9\runtime\jcl\common\jclexception.c(413): error C2220: warning treated as error - no 'object' file generated

[2021-01-26T17:05:06.306Z] F:\Users\jenkins\workspace\Build_JDK11_x86-64_windows_Personal\openj9\runtime\jcl\common\jclexception.c(413): warning C4244: '=': conversion from 'UDATA' to 'U_32', possible loss of data

[2021-01-26T17:05:06.306Z] F:\Users\jenkins\workspace\Build_JDK11_x86-64_windows_Personal\openj9\runtime\jcl\common\jclexception.c(414): warning C4244: '=': conversion from 'UDATA' to 'U_32', possible loss of data

[pshipton]

jenkins compile win,osx jdk11

[pshipton]

I think this is good now. Thanks for all your hard work getting this right @sharon-wang