Hello Phil
The change looks good - Reduces the number of local references held by the JNI
function.
Here are some findings :
1. I found additional references that could be released. You can check the
severity of the problem and decide to release or skip.
Line: 1129 Concerned Object: cacheDirArray.
I believe the existing code may not create trouble as
reference to object is outside the loop.
Line: 1168 and
Line: 1175 Concerned Object: fcCompFontObj
Both these lines, either terminate /or continue the
loop without releasing the reference.
Line: 1330 Concerned Object: fcFont
When jstr becomes NULL, for loop is exited with break
without releasing the reference
Line: 1364 Concerned Object: fcFontArr
fcFontArr (new object array ) is created in every
iteration of the loop.
This could be released in this line alongside release
of fcCompFontObj
2. Copyright notice to have 2017 instead of 2016
3. Question: Should we mask the other warning messages with 'grep -v' in
JNICheck.sh ?
Thanks
Have a good day
Prahalad N.
------------------------------
Message: 2
Date: Tue, 17 Jan 2017 12:29:24 -0800
From: Phil Race <[email protected]>
To: Sergey Bylokhov <[email protected]>
Cc: 2d-dev <[email protected]>
Subject: Re: [OpenJDK 2D-Dev] RFR: 8172813
test/java/awt/font/JNICheck/JNICheck.sh fails on Linux
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Both yes and no :-), although I think it is fine to add both.
1140 .. there are no more than 2 cachedirs (1 user + 1 system) so I didn't
examine JNI refs in this loop
1166 .. I was not realistically expecting a "NULL" here so ignored it.
Unlike some of the JNI bugs "call while pending exception" type bugs we've
fixed these were actual, not hypothetical.
Updated webrev:
http://cr.openjdk.java.net/~prr/8172813.1
-phil.
On 01/16/2017 09:07 AM, Sergey Bylokhov wrote:
> Hi, Phil.
> Should we add DeleteLocalRef in fontpath.c at lines 1140 and 1166, or both
> were skipped intentionally?
>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8172813
>> Webrev : http://cr.openjdk.java.net/~prr/8172813/
>>
>> - Clear local refs once we are done with them.
>> - Use grep to filter out from the log unrelated warnings as separately
>> reported here :
>>
>> https://bugs.openjdk.java.net/browse/JDK-8131136
>>
>> -phil.
