<AWT Dev> Review Request For 8146230: Crash in JNI_ArgumentPusherVaArg::JNI_ArgumentPusherVaArg(_jmethodID*, __va_list_tag*)+0xa

[Ambarish Rapte]

Hi Semyon,

 

                1. The _initIDs() functions are called from static initialization block of particular class, hence these functions would be called when class is getting loaded.

     The _initIDs() functions are used to initialize the IDs & get value of Java class members only.

                                For example:

Java_sun_awt_X11_XToolkit_initIDs()  initializes IDs and gets value from java side XToolkit class (XToolkit.java)

 

                2.  But  awtJNI_ThreadYield()  is late initialization of IDs. IDs are initialized when first thread has to be yield.

 

ð   Addition of operations to Java_sun_awt_X11_XToolkit_initIDs() is addition to class load time.

ð   XToolkit.c::get_xawt_root_shell()  is another example of late initialization of jclass, jmethodID.

 

 

Regards,

Ambarish

 

From: Semyon Sadetsky 
Sent: Wednesday, July 06, 2016 11:39 PM
To: Ambarish Rapte; Sergey Bylokhov; Alexander Scherbatiy; Prasanta Sadhukhan; awt-dev at openjdk.java.net
Subject: Re: Review Request For 8146230: Crash in JNI_ArgumentPusherVaArg::JNI_ArgumentPusherVaArg(_jmethodID*, __va_list_tag*)+0xa

 

Hi Ambarish,

Why not simply initialize yieldMethodID separately? For example in Java_sun_awt_X11_XToolkit_initIDs.

--Semyon

On 06.07.2016 14:23, Ambarish Rapte wrote:

Hi,

 

                Please review the fix for JDK9,

                Bug: https://bugs.openjdk.java.net/browse/JDK-8146230

                Webrev: http://cr.openjdk.java.net/~arapte/8146230/webrev.00/

 

 

Issue:

1.       Null pointer exception in JNI

 

Cause:

The code block was not multi thread safe.

Issue occurs in  multi threaded , multi processor environment.

 

Fix:

1.       Changed the variable used for double checking, to use  yieldMethodID .

2.       Changed yieldMethodID  to  volatile.

3.       Added AWT_LOCK() over initialize block of code.

4.       Removed unrequired  err variable.

 

A drawback of Double Check Locking ( DCL ) is, if the resource assignment is not an atomic operation, The DCL may fail.

But here in the code of concern, is an atomic operation. Hence DCL should work fine.

Please check below reference link for more detailed discussion of DCL with C++.

 

 

Verification:

1.       Tested Event tests which pass without any regression of this change.

2.       As this change only corrects existing logic, there should be no side effects.

 

 

Reference:

                C++ and the Perils of Double-Checked Locking:  http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf

 

 

Regards,

Ambarish

 

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160707/2d8ea43e/attachment-0001.html>