RFR: 8085903: New fix for memory leak in ProtectionDomain cache
Sean Mullan
sean.mullan at oracle.com
Tue Jan 12 16:02:43 UTC 2016
On 01/12/2016 10:26 AM, Xuelei Fan wrote:
> I think hashMap.put() may be similar or more effective than
> hashMap.putIfAbsent().
Ok, I think I see what you are trying to say.
The common case is that the putIfAbsent method is going to succeed (i.e.
the entry will be absent). It may not succeed in cases where there are
multiple threads simultaneously computing the initial permissions for
the same PD or the value is garbage collected due to memory demand.
So, I think the question boils down to whether put is faster than
putIfAbsent in the common case (when there is not an entry). Probably
yes, since it doesn't have to check if there is an entry already.
> Is there a case that the key/weakPd is the same,
> but the value/pc is different?
I believe the pc should always be the same. If you change the policy
file(s), then you have to explicitly refresh it (using Policy.refresh())
which should cause a new cache to be created.
Let me do some more testing around this, and see if I can check
performance and see what is better.
Thanks,
Sean
>
> Xuelei
>
> On 1/12/2016 11:01 PM, Sean Mullan wrote:
>> Hi Ivan,
>>
>> On 01/12/2016 09:38 AM, Ivan Gerasimov wrote:
>>> Hi Sean!
>>>
>>> On 12.01.2016 16:26, Sean Mullan wrote:
>>>> I received a private comment that there may be cases where the map's
>>>> value is reclaimed by the garbage collector, but the key still exists.
>>>> If that ProtectionDomain is subsequently used for permission checks, a
>>>> Map.putIfAbsent method will not replace the value.
>>>>
>>>> So, I have added an additional check for this case in the PDCache.put
>>>> method, and it instead uses the Map.replace method. Updated webrev:
>>>>
>>>
>>> Wouldn't it be a little bit more efficient to use merge() here?
>>>
>>> Something like:
>>> pdMap.merge(weakPd, v, (pv, nv) -> pv.get() != null ? pv : nv);
>>
>> Yes - good catch!
>>
>> Will update and send out new webrev after a round of testing ...
>>
>> Thanks,
>> Sean
>>
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>>> http://cr.openjdk.java.net/~mullan/webrevs/8085903/webrev.01/
>>>>
>>>> --Sean
>>>>
>>>> On 01/08/2016 03:06 PM, Sean Mullan wrote:
>>>>> Please review this fix for a memory leak in the ProtectionDomain cache
>>>>> (which maps each ProtectionDomain to their granted
>>>>> PermissionCollection). The memory leak only occurs if custom
>>>>> permissions
>>>>> are used in a policy file.
>>>>>
>>>>> http://cr.openjdk.java.net/~mullan/webrevs/8085903/webrev.00/
>>>>>
>>>>> Custom permissions cause a chain of strong references that link back to
>>>>> the ProtectionDomain key used in the map, preventing it from being
>>>>> removed (and also preventing the custom permission's URLClassLoader
>>>>> from
>>>>> being garbage collected). This fix wraps the PermissionCollection in a
>>>>> SoftReference which allows the objects to be garbage collected in
>>>>> response to memory demand.
>>>>>
>>>>> Thanks,
>>>>> Sean
>>>>
>>>
>
More information about the security-dev
mailing list