On 07.07.2016 16:14, Ambarish Rapte wrote:
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)
awtJNI_ThreadYield() is only used by the XToolkit. I don't see anything
bad if it will be initialized there.
2. But awtJNI_ThreadYield() is late initialization
of IDs. IDs are initialized when first thread has to be yield.
Is lazy initialization really necessary? It looks like the method ID is
used always.
ð Addition of operations to Java_sun_awt_X11_XToolkit_initIDs() is
addition to class load time.
Lock also introduces delay. Also it maybe a source for deadlocks.
Locking toolkit thread using global lock object should be really justified.
ð XToolkit.c::get_xawt_root_shell() is another example of late
initialization of jclass, jmethodID.
Should it be fixed as well?
--Semyon
Regards,
Ambarish
*From:*Semyon Sadetsky
*Sent:* Wednesday, July 06, 2016 11:39 PM
*To:* Ambarish Rapte; Sergey Bylokhov; Alexander Scherbatiy; Prasanta
Sadhukhan; awt-dev@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/
<http://cr.openjdk.java.net/%7Earapte/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