On Thu, 18 Feb 2021 13:31:07 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> 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)
> 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.
Oh - right - still - if you'd stick with that fix - I believe you should do
that:
try {
// If the class is present, it will be initialized which
// triggers registration of the extended socket options.
Class<?> c = Class.forName("jdk.net.ExtendedSocketOptions");
ext = instance;
} catch (ClassNotFoundException e) {
// the jdk.net module is not present => no extended socket
options
ext = instance = new NoExtendedSocketOptions();
}
}
return ext;
```
Anyway: waiting for the next fix :-)
-------------
PR: https://git.openjdk.java.net/jdk/pull/2601
