[
https://issues.apache.org/jira/browse/SLING-2786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13600974#comment-13600974
]
Bertrand Delacretaz commented on SLING-2786:
--------------------------------------------
I agree with the modifications that you suggest.
And of course you're right about locking keySet etc. after clients have
acquired copies of them.
If using inner class implementations is not too problematic, why not...we could
also just document in the javadocs that the return values of entrySet() etc.
should never be stored, to avoid that problem. I don't think it's a common use
case so documenting might be enough.
> ResourceMetadata contains stale unmodifiableMap cache after clone()
> -------------------------------------------------------------------
>
> Key: SLING-2786
> URL: https://issues.apache.org/jira/browse/SLING-2786
> Project: Sling
> Issue Type: Bug
> Components: API
> Affects Versions: API 2.3.0
> Reporter: Lukas Eder
> Priority: Minor
> Attachments: ResourceMetadataTest.java.patch
>
>
> Super types of ResourceMetadata (java.util.HashMap and java.util.AbstractMap)
> correctly implement caching for members such as entrySet, keySet, values,
> etc. Two things should be considered:
> 1. The cache should be marked transient. It is not desireable to serialise /
> deserialise the "unmodifiableMap" cache. Only the isReadOnly flag should be
> serialised
> 2. clone() should be overridden in order to reset the cache on cloned
> instances
> Consider the following code:
> ResourceMetadata map1 = new ResourceMetadata();
> map1.put("key1", "value1");
> map1.lock();
> map1.values(); // Enforce the creation of the cache
> ResourceMetadata map2 = (ResourceMetadata) map1.clone();
> Now, map2 contains an unmodifiable wrapper of map1. While this generally
> behaves correctly, there are two problems:
> a) This is a potential memory leak as clones hold a reference to the original
> map.
> b) If unlock() is ever added, this might lead to a subtle bug when (after the
> above code):
> - map1.unlock() is called
> - map1 is modified
> - map2 will reflect map1's modifications, even if map2 should still be
> locked
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira