Code Review Request for 7107613, 7107616, 7185471

[Weijun Wang]

On 07/27/2012 07:26 AM, Valerie (Yu-Ching) Peng wrote:
> Max,
>
> So, do you want me to check the exemptCache again, i.e. what line 122-
> 125 do, inside the synchronized block before calling
> getAppPermissions(...)?

Yes, that should work.

If you do believe this is a very rare case and you don't mind 
getAppPermissions() called twice, the synchonized block seems 
unnecessary. Both getAppPermissions and putIfAbsent should be 
thread-safe methods.

Thanks
Max

>
> Thanks,
> Valerie
>
> On 07/25/12 18:29, Weijun Wang wrote:
>>> http://cr.openjdk.java.net/~valeriep/7107616
>>
>> You have
>>
>>  122         CryptoPermissions appPerms =
>> exemptCache.get(callerCodeBase);
>>  123         // Found result in cache
>>  124         if (appPerms != null) {
>>  125             if (appPerms == CACHE_NULL_MARK) appPerms = null;
>>  126         } else {
>>  127             synchronized (this.getClass()) {
>>  128                 appPerms = getAppPermissions(callerCodeBase);
>>  129                 exemptCache.putIfAbsent(callerCodeBase,
>>  130                     (appPerms == null? CACHE_NULL_MARK:appPerms));
>>  131             }
>>  132         }
>>
>> This is not as optimized as before, that there is a chance
>> getAppPermissions could be called twice, and this seems to make the
>> synchronized block not worth syncing.
>>
>
>>> http://cr.openjdk.java.net/~valeriep/7185471
>>
>> I haven't looked at this yet. Seems a lot of math there.
>>
>> Thanks
>> Max
>>
>>>
>>> The changes are for JDK 8. May be backported to 7u later if necessary,
>>> Thanks,
>>> Valerie
>