On Thu, 18 Feb 2021 13:23:37 GMT, Vyom Mani Tewari
<github.com+4410404+vyomm...@openjdk.org> wrote:
>> Hello Daniel, I had thought about it in my previous commit. But this won't
>> work, since in the normal case where the `ClassNotFoundException` doesn't
>> get thrown, the `instance` is actually set in the `register` method which
>> gets triggered due to the class load on `jdk.net.ExtendedSocketOptions`. As
>> a result, returning the local `ext` variable won't work in that case, unless
>> of course I do `ext = instance` in both the catch block and outside of it,
>> which would, IMO, defeat the purpose of optimization in that last commit.
>>
>> I decided to "prove" it with some test case and while doing so I just
>> uncovered that my whole patch has just moved the deadlock to a new location
>> - thread T1 calling `sun.net.ext.ExtendedSocketOptions#getInstance()` and
>> thread T2 calling `Class.forName("jdk.net.ExtendedSocketOptions")` ends up
>> in a deadlock. It's clears why that happens.
>>
>> I am going to take a step back and come back with a different fix for this
>> one. Thank you everyone for these reviews - they definitely helped.
>
> Hi Jaikiran,
>
> I tested with my suggested change and i did not see any deadlock at my local
> Linux environment. I just ran test in loop and it worked as expected.
>
> Thanks,
> Vyom
Hello Vyom,
The trick is to change the test case to have something like this in the "task":
diff --git a/test/jdk/sun/net/ext/ExtendedSocketOptionsTest.java
b/test/jdk/sun/net/ext/ExtendedSocketOptionsTest.java
index 0702abf5279..26c8a1384a2 100644
--- a/test/jdk/sun/net/ext/ExtendedSocketOptionsTest.java
+++ b/test/jdk/sun/net/ext/ExtendedSocketOptionsTest.java
@@ -96,7 +96,11 @@ public class ExtendedSocketOptionsTest {
classLoadingTriggerLatch.countDown();
// wait for the other task to let us know it's ready too, to
load the class
classLoadingTriggerLatch.await();
- return Class.forName(this.className);
+ final Class<?> c = Class.forName(this.className);
+ // let's call getInstance on sun.net.ext.ExtendedSocketOptions
+ final Class<?> k =
Class.forName("sun.net.ext.ExtendedSocketOptions");
+ final Object extSocketOptions =
k.getDeclaredMethod("getInstance").invoke(null);
+ return c;
} catch (Exception e) {
System.err.println("Failed to load " + this.className);
throw new RuntimeException(e);
Essentially, trigger a call to `sun.net.ext.ExtendedSocketOptions#getInstance`
and a classload of `jdk.net.ExtendedSocketOptions` simultaneously from
different threads. That will end up with:
"pool-1-thread-1" #16 prio=5 os_prio=31 cpu=18.25ms elapsed=120.03s
tid=0x00007ff9008c8a00 nid=0x6203 waiting for monitor entry
[0x0000700010c8b000]
java.lang.Thread.State: BLOCKED (on object monitor)
at
sun.net.ext.ExtendedSocketOptions.register(java.base@17-internal/ExtendedSocketOptions.java:197)
- waiting to lock <0x000000070f9f8e68> (a java.lang.Class for
sun.net.ext.ExtendedSocketOptions)
at
jdk.net.ExtendedSocketOptions.<clinit>(jdk.net@17-internal/ExtendedSocketOptions.java:234)
at java.lang.Class.forName0(java.base@17-internal/Native Method)
at java.lang.Class.forName(java.base@17-internal/Class.java:375)
at
ExtendedSocketOptionsTest$Task.call(ExtendedSocketOptionsTest.java:100)
at
ExtendedSocketOptionsTest$Task.call(ExtendedSocketOptionsTest.java:84)
at
java.util.concurrent.FutureTask.run(java.base@17-internal/FutureTask.java:264)
at
java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@17-internal/ThreadPoolExecutor.java:1135)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@17-internal/ThreadPoolExecutor.java:635)
at java.lang.Thread.run(java.base@17-internal/Thread.java:831)
"pool-1-thread-2" #17 prio=5 os_prio=31 cpu=8.76ms elapsed=120.03s
tid=0x00007ff90102be00 nid=0x6403 in Object.wait() [0x0000700010d8e000]
java.lang.Thread.State: RUNNABLE
at java.lang.Class.forName0(java.base@17-internal/Native Method)
- waiting on the Class initialization monitor for
jdk.net.ExtendedSocketOptions
at java.lang.Class.forName(java.base@17-internal/Class.java:375)
at
sun.net.ext.ExtendedSocketOptions.getInstance(java.base@17-internal/ExtendedSocketOptions.java:185)
- locked <0x000000070f9f8e68> (a java.lang.Class for
sun.net.ext.ExtendedSocketOptions)
at
jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@17-internal/Native
Method)
at
jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@17-internal/NativeMethodAccessorImpl.java:78)
at
jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@17-internal/DelegatingMethodAccessorImpl.java:43)
at
java.lang.reflect.Method.invoke(java.base@17-internal/Method.java:566)
at
ExtendedSocketOptionsTest$Task.call(ExtendedSocketOptionsTest.java:102)
at
ExtendedSocketOptionsTest$Task.call(ExtendedSocketOptionsTest.java:84)
at
java.util.concurrent.FutureTask.run(java.base@17-internal/FutureTask.java:264)
at
java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@17-internal/ThreadPoolExecutor.java:1135)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@17-internal/ThreadPoolExecutor.java:635)
at java.lang.Thread.run(java.base@17-internal/Thread.java:831)
-------------
PR: https://git.openjdk.java.net/jdk/pull/2601
