+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 <[email protected]>; [email protected]
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: [email protected]
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.
