On Thu, 18 Feb 2021 13:05:48 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> src/java.base/share/classes/sun/net/ext/ExtendedSocketOptions.java line 189:
>> 
>>> 187:             }
>>> 188:         }
>>> 189:         return instance;
>> 
>> I'd suggest changing these two lines as well: 
>> 
>>                 // the jdk.net module is not present => no extended socket 
>> options
>>                 ext = instance = new NoExtendedSocketOptions();
>>             }
>>         }
>>         return ext;
>
> 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

-------------

PR: https://git.openjdk.java.net/jdk/pull/2601

Reply via email to