[
https://issues.apache.org/jira/browse/AVRO-607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14387777#comment-14387777
]
Ryan Blue commented on AVRO-607:
--------------------------------
[~davelatham]'s comment is a good excuse to start this thread again. From the
discussion above, it sounds like building the WeakConcurrentMap is a problem
because its equals method doesn't override the default and two WeakReferences
to the same object aren't equal. But, WeakIdentityHashMap creates a subclass of
WeakReference that overrides equals to avoid that problem and it would be
appropriate to use an IdentityHashMap for this cache. So I created a version of
the WeakIdentityHashMap that is backed by a ConcurrentHashMap instead of a
regular HashMap. Even if the GC removes the weakly-referenced object while the
ConcurrentHashMap is doing some operation, it either removes all weak
references or none, so key equality is preserved.
I also caught a couple of places where I think the current implementation
violates its contract. For example, {{keySet}} gets all of the identity weak
references and accumulates a set of the real values. As soon as each real value
is referenced, we know that the weak reference won't disappear. But I think
there's a small opportunity between reaping the current reference set and
adding all the referenced objects to the key set for GC to run and remove
values:
{code:java}
public Set<K> keySet() {
reap();
Set<K> ret = new HashSet<K>();
for (IdentityWeakReference<K> ref : backingStore.keySet()) {
// half-way through the loop, GC could run and remove the next referenced
object
ret.add(ref.get());
}
return Collections.unmodifiableSet(ret);
}
{code}
I've also added checks so that in {{keySet}} and {{entrySet}}, the
WeakReference is resolved (which prevents losing the value) and checked to see
if it is null before adding to the output collection.
I've added [PR #30|https://github.com/apache/avro/pull/30] on github, and I can
upload a patch if that's preferred.
> SpecificData.getSchema not thread-safe
> --------------------------------------
>
> Key: AVRO-607
> URL: https://issues.apache.org/jira/browse/AVRO-607
> Project: Avro
> Issue Type: Bug
> Components: java
> Affects Versions: 1.3.3
> Reporter: Stephen Tu
> Attachments: AVRO-607.patch
>
>
> SpecificData.getSchema uses a WeakHashMap to cache schemas, but WeakHashMap
> is not thread-safe, and the method itself is not synchronized. Seems like
> this could lead to the data structure getting corrupted.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)