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

[Semyon Sadetsky]

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 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/
>     <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
>

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