[OpenJDK 2D-Dev] [9] Review Request- JDK-8152971- JNI Warning with -Xcheck:jni

[Prasanta Sadhukhan]

I guess this method(s) should take "fontStrLC" instead of "fontStr"

650             (*env)->CallObjectMethod(env, fontToFileMap, fmi->putMID,
  651                                      fontStr, fileStr);

  692         (*env)->CallObjectMethod(env, fontToFileMap, fmi->putMID,
  693                                  fontStr, fileStr);

762             (*env)->CallObjectMethod(env, fontToFileMap, fmi->putMID,
  763                                      fontStr, fileStr);

805         (*env)->CallObjectMethod(env, fontToFileMap, fmi->putMID,
  806                                  fontStr, fileStr);

It seems this line is not needed as we have not populated fmi structure yet.

882 releaseGdiFontMapReferences(env, &fmi); Why aren't we deleting 
fmi->env,fmi.arrayListCtr,fmi.addMID,fmi.putMID in 
releaseGdiFontMapReferences()? Also, it seems earlier the testscript was 
supposed to execute only on Mac but now you are extending the execution 
platform to windows and linux as well excluding only solaris. Is there 
any particular need to restrict solaris too? Regards Prasanta

On 6/24/2016 7:34 PM, Prahalad Kumar Narayanan wrote:
> RE: [9] Review Request- JDK-8152971- JNI Warning with -Xcheck:jni
>
> Hello Everyoneon Java2D Forum
>
> Good day to you.
>
> A Quick follow-up on webrev to fix the following issue
>
>       Bug ID     : JDK-8152971  / -Xcheck:jni - warning in native method
>
>       Bug Link : 
> https://bugs.openjdk.java.net/browse/JDK-8152971?filter=-1
>
> Updated webrev 
> link:_http://cr.openjdk.java.net/~pnarayanan/8152971/webrev.01/_<http://cr.openjdk.java.net/%7Epnarayanan/8152971/webrev.01/>
>
> Description on Changes
>
>       .The previous webrevcontained changes 
> toadditionalfileswhicharenot related to the current bug in concern.
>
>       . Henceforth, the updated webrev limits the changes to only 
> fontpath.c and a Jtreg test script to verify the change.
>
> RegardingBuild & Test
>
>       .Though the changes pertain to windows specific code,internal 
> buildsystemwastriggeredto ensure safe build on all supportedplatforms.
>
>       .In addition,no new Jtreg failureswere found withthe proposed 
> changes.
>
> Kindly review the changes at your convenience & provide your feedback.
>
> Thank you for your time in review
>
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Prahalad Kumar Narayanan
> Sent: Wednesday, June 22, 2016 3:20 PM
> To: 2d-dev at openjdk.java.net
> Cc: Philip Race; Prasanta Sadhukhan; Jayathirth D V; Praveen Srivastava
> Subject: [9] Review Request- JDK-8152971- JNI Warning with -Xcheck:jni
>
> Hello Everyone, on Java2D Group
>
> Good day to you.
>
> Please find herewith, webrev changes to fix the bug-
>
>       Bug ID     : JDK-8152971  / -Xcheck:jni - warning in native method
>
>       Bug Link :https://bugs.openjdk.java.net/browse/JDK-8152971?filter=-1
>
>       Webrev 
> :http://cr.openjdk.java.net/~pnarayanan/8152971/webrev.00/<http://cr.openjdk.java.net/%7Epnarayanan/8152971/webrev.00/>
>
> Description on Bug:
>
>      1.  Too many JNI warnings are reported in the native functions 
> when test-code is run with VM Option:  -Xcheck:jni
>
>      2.  The warnings can be classified into 2 categories
>
> a.  JNI warnings that are thrown due to the missing exception checks 
> after an Up call ( JNI function invoking Java method )
>
> b.  JNI warnings that are thrown due to increased usage of Local 
> Reference objects.
>
> Description on the Fix:
>
>      1.  File : FontPath.c
>
> Added JNIEnv->ExceptionCheck() and ExceptionClear() where Up call is 
> invoked.
>
> The checks are added only to the valid Up calls as per JNI spec.
>
> Added JNIEnv->DeleteLocalRef where the native functions created Java 
> side objects but did not delete their local reference after usage.
>
> A few of the native functions get invoked as Callbacks to Windows APIs 
> following the font enumeration.
>
> Therefore at each callback, the count of active local references would 
> increase.
>
> Over time, the active local references would exceed the planned number 
> of local references set by JVM.
>
>      2.  File : awt_Component.cpp
>
> Added JNIEnv->ExceptionCheck() and ExceptionClear() where an Up call 
> displayed a JNI warning while running the Jtreg test script.
>
> Information on Jtreg test script is given below.
>
>      3.  File : LoadFontsJNICheck.sh
>
> The shell script is already a part of JTreg test case & invokes 
> LoadFontsJNICheck with -Xcheck:jni VM option
>
> However, the script was configured to run only on Mac OS. Now, the 
> script is modified to run on windows, linux and mac systems.
>
> This way, the code changes can be checked for proper execution with 
> test-case.
>
> Kindly review the changes at your convenience and share your feedback.
>
> Thank you for your time in review
>
> Have a good day
>
> Prahalad N.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160627/d25d5b61/attachment.html>