[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8253280: Use class name as class loading lock
From:       David Holmes <david.holmes () oracle ! com>
Date:       2020-11-16 5:55:59
Message-ID: d682a617-f668-a0e9-e751-4041216740f7 () oracle ! com
[Download RAW message or body]

Hi Robert,

I have to agree with Mandy and Alan here. What you propose, in terms of 
getting more visibility into which class the classloader is trying to 
load, is quite reasonable. But the mechanism is quite problematic for a 
number of reasons as Mandy mentioned. In particular platform code needs 
to avoid calling toString() on user-defined objects as it can do anything.

Also note that java.lang.String is not a good example here as you should 
never be loading it (or any core class) explicitly via a user-defined 
classloader.

On 15/11/2020 2:06 am, Robert LU wrote:
> When many thread try to load same class, the thread will stuck on \
> `ClassLoader.loadClass`. At current jdk, the stacktrace by example program is:
> "Thread-1" prio=5 Id=13 BLOCKED on java.lang.String@724af044 owned by "Thread-0" \
> Id=12 at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:646)
>                 
> -  blocked on java.lang.String@724af044 val="java.lang.String"

This seems to be the after case, not before.

Cheers,
David
-----


> at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:634)
>  at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:182)
>  at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
> at app//Main2.test(Main2.java:19)
> at app//Main2$$Lambda$37/0x00000001000c2a20.run(Unknown Source)
> at java.base/java.lang.Thread.run(Thread.java:831)
> There is no way to get which class stuck the thread.
> 
> **After this patch, the stacktrace will be**:
> "Thread-2" prio=5 Id=13 BLOCKED on java.lang.String@724af044 owned by "Thread-3" \
> Id=14  at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:612)
>                 
> 	-  blocked on java.lang.String@724af044 val="java.lang.String"
> 	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:600)
>   at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
>   at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
> 	at app//Main2.test(Main2.java:18)
> 	at app//Main2$$Lambda$38/0x0000000100097440.run(Unknown Source)
> 	at java.base/java.lang.Thread.run(Thread.java:832)
> That is, user will know which class stuck the thread, in this example, the class is \
> `java.lang.String`. It's helpful for troubleshooting. 
> The example program:
> Before patch:
> <details>
> <summary>Main.java</summary>
> 
> // Main.java
> import java.io.PrintStream;
> import java.lang.management.*;
> 
> public final class Main {
> private synchronized static void test1() {
> while (true) {
> try {
> Thread.sleep(1000);
> } catch (InterruptedException e) {
> e.printStackTrace();
> }
> }
> }
> 
> private static void test() {
> while (true) {
> try {
> Main.class.getClassLoader().loadClass("java.lang.String");
> } catch (ClassNotFoundException e) {
> }
> }
> }
> 
> public static void main(String[] args) throws InterruptedException, \
> NoSuchFieldException, IllegalAccessException { new Thread(Main::test).start();
> new Thread(Main::test).start();
> new Thread(Main::test).start();
> new Thread(Main::test).start();
> new Thread(Main::test).start();
> 
> while (true) {
> Thread.sleep(1000);
> ThreadMXBean bean = ManagementFactory.getThreadMXBean();
> ThreadInfo[] infos = bean.dumpAllThreads(true, true);
> for (ThreadInfo info : infos) {
> System.out.println(printThreadInfo(info));
> }
> }
> }
> 
> private static String printThreadInfo(ThreadInfo threadInfo) {
> StringBuilder sb = new StringBuilder(""" + threadInfo.getThreadName() + """ +
> (threadInfo.isDaemon() ? " daemon" : "") +
> " prio=" + threadInfo.getPriority() +
> " Id=" + threadInfo.getThreadId() + " " +
> threadInfo.getThreadState());
> if (threadInfo.getLockName() != null) {
> sb.append(" on " + threadInfo.getLockName());
> }
> if (threadInfo.getLockOwnerName() != null) {
> sb.append(" owned by "" + threadInfo.getLockOwnerName() +
> "" Id=" + threadInfo.getLockOwnerId());
> }
> if (threadInfo.isSuspended()) {
> sb.append(" (suspended)");
> }
> if (threadInfo.isInNative()) {
> sb.append(" (in native)");
> }
> sb.append('\n');
> int i = 0;
> StackTraceElement[] stackTrace = threadInfo.getStackTrace();
> for (; i < stackTrace.length; i++) {
> StackTraceElement ste = stackTrace[i];
> sb.append("\tat " + ste.toString());
> sb.append('\n');
> if (i == 0 && threadInfo.getLockInfo() != null) {
> Thread.State ts = threadInfo.getThreadState();
> switch (ts) {
> case BLOCKED:
> sb.append("\t-  blocked on " + printLockInfo(threadInfo.getLockInfo()));
> sb.append('\n');
> break;
> case WAITING:
> sb.append("\t-  waiting on " + printLockInfo(threadInfo.getLockInfo()));
> sb.append('\n');
> break;
> case TIMED_WAITING:
> sb.append("\t-  waiting on " + printLockInfo(threadInfo.getLockInfo()));
> sb.append('\n');
> break;
> default:
> }
> }
> 
> for (MonitorInfo mi : threadInfo.getLockedMonitors()) {
> if (mi.getLockedStackDepth() == i) {
> sb.append("\t-  locked " + printLockInfo(mi));
> sb.append('\n');
> }
> }
> }
> if (i < stackTrace.length) {
> sb.append("\t...");
> sb.append('\n');
> }
> 
> LockInfo[] locks = threadInfo.getLockedSynchronizers();
> if (locks.length > 0) {
> sb.append("\n\tNumber of locked synchronizers = " + locks.length);
> sb.append('\n');
> for (LockInfo li : locks) {
> sb.append("\t- " + printLockInfo(li));
> sb.append('\n');
> }
> }
> sb.append('\n');
> return sb.toString();
> }
> 
> private static String printLockInfo(LockInfo li) {
> String res = li.getClassName() + '@' + \
> Integer.toHexString(li.getIdentityHashCode()); return res;
> }
> }
> </details>
> After patch:
> <details>
> <summary>Main2.java</summary>
> 
> // Main2.java
> import java.io.PrintStream;
> import java.lang.management.*;
> 
> public final class Main2 {
> private synchronized static void test1() {
> while (true) {
> try {
> Thread.sleep(1000);
> } catch (InterruptedException e) {
> e.printStackTrace();
> }
> }
> }
> 
> private static void test() {
> while (true) {
> try {
> Main2.class.getClassLoader().loadClass("java.lang.String");
> } catch (ClassNotFoundException e) {
> }
> }
> }
> 
> public static void main(String[] args) throws InterruptedException, \
> NoSuchFieldException, IllegalAccessException { new Thread(Main2::test).start();
> new Thread(Main2::test).start();
> new Thread(Main2::test).start();
> new Thread(Main2::test).start();
> new Thread(Main2::test).start();
> 
> while (true) {
> Thread.sleep(1000);
> ThreadMXBean bean = ManagementFactory.getThreadMXBean();
> ThreadInfo[] infos = bean.dumpAllThreads(true, true);
> for (ThreadInfo info : infos) {
> System.out.println(printThreadInfo(info));
> }
> }
> }
> 
> private static String printThreadInfo(ThreadInfo threadInfo) {
> StringBuilder sb = new StringBuilder(""" + threadInfo.getThreadName() + """ +
> (threadInfo.isDaemon() ? " daemon" : "") +
> " prio=" + threadInfo.getPriority() +
> " Id=" + threadInfo.getThreadId() + " " +
> threadInfo.getThreadState());
> if (threadInfo.getLockName() != null) {
> sb.append(" on " + threadInfo.getLockName());
> }
> if (threadInfo.getLockOwnerName() != null) {
> sb.append(" owned by "" + threadInfo.getLockOwnerName() +
> "" Id=" + threadInfo.getLockOwnerId());
> }
> if (threadInfo.isSuspended()) {
> sb.append(" (suspended)");
> }
> if (threadInfo.isInNative()) {
> sb.append(" (in native)");
> }
> sb.append('\n');
> int i = 0;
> StackTraceElement[] stackTrace = threadInfo.getStackTrace();
> for (; i < stackTrace.length; i++) {
> StackTraceElement ste = stackTrace[i];
> sb.append("\tat " + ste.toString());
> sb.append('\n');
> if (i == 0 && threadInfo.getLockInfo() != null) {
> Thread.State ts = threadInfo.getThreadState();
> switch (ts) {
> case BLOCKED:
> sb.append("\t-  blocked on " + printLockInfo(threadInfo.getLockInfo()));
> sb.append('\n');
> break;
> case WAITING:
> sb.append("\t-  waiting on " + printLockInfo(threadInfo.getLockInfo()));
> sb.append('\n');
> break;
> case TIMED_WAITING:
> sb.append("\t-  waiting on " + printLockInfo(threadInfo.getLockInfo()));
> sb.append('\n');
> break;
> default:
> }
> }
> 
> for (MonitorInfo mi : threadInfo.getLockedMonitors()) {
> if (mi.getLockedStackDepth() == i) {
> sb.append("\t-  locked " + printLockInfo(mi));
> sb.append('\n');
> }
> }
> }
> if (i < stackTrace.length) {
> sb.append("\t...");
> sb.append('\n');
> }
> 
> LockInfo[] locks = threadInfo.getLockedSynchronizers();
> if (locks.length > 0) {
> sb.append("\n\tNumber of locked synchronizers = " + locks.length);
> sb.append('\n');
> for (LockInfo li : locks) {
> sb.append("\t- " + printLockInfo(li));
> sb.append('\n');
> }
> }
> sb.append('\n');
> return sb.toString();
> }
> 
> private static String printLockInfo(LockInfo li) {
> String res = li.getClassName() + '@' + \
> Integer.toHexString(li.getIdentityHashCode()); // There is no getLock method in \
> current jdk if (li.getStringValue() != null) {
> return res + " val="" + li.getStringValue() + """;
> }
> return res;
> }
> }
> </details>
> 
> -------------
> 
> Commit messages:
> - 8253280: Use class name as class loading lock
> 
> Changes: https://git.openjdk.java.net/jdk/pull/104/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=104&range=00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8253280
> Stats: 73 lines in 5 files changed: 72 ins; 0 del; 1 mod
> Patch: https://git.openjdk.java.net/jdk/pull/104.diff
> Fetch: git fetch https://git.openjdk.java.net/jdk pull/104/head:pull/104
> 
> PR: https://git.openjdk.java.net/jdk/pull/104
> 


[prev in list] [next in list] [prev in thread] [next in thread]