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

List:       openjdk-awt-dev
Subject:    Re: <AWT Dev> [11] Review Request: 8189604 possible hang in sun.awt.shell.Win32ShellFolder2$KnownFol
From:       Krishna Addepalli <krishna.addepalli () oracle ! com>
Date:       2018-06-26 9:05:30
Message-ID: b6824fc7-c7b2-418f-96cd-4d7dbe5d4d0b () default
[Download RAW message or body]

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@oracle.com>; awt-dev@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@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.


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