<AWT Dev> [11] Review Request: 8189604 possible hang in sun.awt.shell.Win32ShellFolder2$KnownFolderDefinition::<clinit>

[Phil Race]

+1, but the bug is missing an evaluation. Please add it.

-phil.

On 06/26/2018 02:05 AM, Krishna Addepalli wrote:
> Hi Sergey,
>
> The changes look fine.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, June 26, 2018 11:12 AM
> To: Krishna Addepalli <krishna.addepalli at oracle.com>; awt-dev at openjdk.java.net
> Subject: Re: <AWT Dev> [11] Review Request: 8189604 possible hang in sun.awt.shell.Win32ShellFolder2$KnownFolderDefinition::<clinit>
>
> Hi, Krishna.
>
> On 25/06/2018 04:52, Krishna Addepalli wrote:
>> The fix looks fine. However, I have couple of comments:
>> 1. The variable "result" in Java_sun_awt_shell_Win32ShellFolder2_loadKnownFolders at line 1401 is not initialized. Although it is initialized in the following if condition, I'm not sure when it return false, and lead to a return of garbage value to Java.
> Yes it should be initialized:
> http://cr.openjdk.java.net/~serb/8189604/webrev.01
>
>> 2. Could you clarify a bit about the testcase - how it triggers the hang/crash?
> The test tries to load all classes using 'Class.forName(name, true, null)"
> which will initialize the classes(will call static initialization).
>
>> Thanks,
>> Krishna
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Monday, June 25, 2018 2:21 PM
>> To: awt-dev at openjdk.java.net
>> Subject: <AWT Dev> [11] Review Request: 8189604 possible hang in
>> sun.awt.shell.Win32ShellFolder2$KnownFolderDefinition::<clinit>
>>
>> Hello.
>> Please review the fix for jdk11.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189604
>> Webrev: http://cr.openjdk.java.net/~serb/8189604/webrev.00
>>
>> We have a cyclic dependency in
>> Win32ShellFolder2$KnownFolderDefinition,
>> which can cause a deadlock if this class is initialized on "non-COM" thread.
>>
>> 1. - Static initialization of Win32ShellFolder2$KnownFolderDefinition
>> 2. -- Win32ShellFolder2.getLibraries(); 3. --- invoke
>> Win32ShellFolder2.loadKnownFolders() on "COM-thread" and wait 4. ----
>> in the native call
>> env->FindClass("sun/awt/shell/Win32ShellFolder2$KnownFolderDefinition"
>> env->);
>> 5. ---- deadlock. because initialization of KnownFolderDefinition was blocked at step 3.
>>
>> In the fix this dependency is broken, so the new class KnownLibraries is not depends from its own initialization.
>>
>>
>
> --
> Best regards, Sergey.