[
https://issues.apache.org/jira/browse/GROOVY-8067?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15853146#comment-15853146
]
ASF GitHub Bot commented on GROOVY-8067:
----------------------------------------
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/489#discussion_r99482851
--- Diff: src/main/org/codehaus/groovy/reflection/ClassInfo.java ---
@@ -186,30 +185,20 @@ public void setStrongMetaClass(MetaClass answer) {
MetaClass strongRef = strongMetaClass;
if (strongRef instanceof ExpandoMetaClass) {
- ((ExpandoMetaClass)strongRef).inRegistry = false;
- synchronized(modifiedExpandos){
- for (Iterator<ClassInfo> it = modifiedExpandos.iterator();
it.hasNext(); ) {
- ClassInfo info = it.next();
- if(info == this){
- it.remove();
- }
+ ((ExpandoMetaClass)strongRef).inRegistry = false;
+ for (Iterator<ClassInfo> itr = modifiedExpandos.iterator();
itr.hasNext(); ) {
+ ClassInfo info = itr.next();
+ if(info == this) {
+ itr.remove();
+ }
}
- }
}
strongMetaClass = answer;
if (answer instanceof ExpandoMetaClass) {
- ((ExpandoMetaClass)answer).inRegistry = true;
- synchronized(modifiedExpandos){
- for (Iterator<ClassInfo> it = modifiedExpandos.iterator();
it.hasNext(); ) {
- ClassInfo info = it.next();
- if(info == this){
- it.remove();
- }
- }
- modifiedExpandos.add(this);
- }
--- End diff --
Removed because it appeared to be a duplicate of the logic above. Only if
the previous previous `strongMetaClass` value was an expando would it have been
added to the map, so the iteration/removal above should have taken care to
remove this `ClassInfo` from the `modifiedExpandos` and there's no reason to
iterate again.
> Possible deadlock when creating new ClassInfo entries in the cache
> ------------------------------------------------------------------
>
> Key: GROOVY-8067
> URL: https://issues.apache.org/jira/browse/GROOVY-8067
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
> Affects Versions: 2.4.8
> Reporter: John Wagenleitner
> Priority: Critical
> Attachments: ClassInfoDeadlockTest.java
>
>
> When running Groovy without {{-Dgroovy.use.classvalue=true}} the ClassInfo
> instances are cached in a {{ManagedConcurrentMap}} (MCM). New values are
> computed on demand and computation involves both a lock on a segment within
> the MCM and a lock on the {{GlobalClassSet}} (GCS) which is backed by a
> {{ManagedLinkedList}}. The problem is that both the ManagedConcurrentMap and
> the GlobalClassSet share the same ReferenceQueue.
> Assume there is an enqueued {{ClassInfo}} value that is stored in Segment2 of
> the MCM. Now assume that Thread1 and Thread2 both request
> {{ClassInfo.getClassInfo(..)}} for two different classes that do not
> currently exist in the cache. Assume that based on hashing Thread1 gets a
> lock on Segment1 and Thread2 gets a lock on Segment2. Assume that Thread1 is
> the first to call computeValue which in turn calls
> {{GlobalClassSet.add(..)}}. This call adds a new value to a
> {{ManagedLinkedList}}, and since it's managed the add operation will process
> the ReferenceQueue. So Thread1 will attempt to dequeue the ClassInfo and the
> finalizeReference method on it's entry will attempt to remove it from
> Segment2. Thread2 holds the lock for Segment2 and Thread2 is blocked and
> can't progress it's waiting on the the lock Thread1 holds the lock for the
> GlobalClassSet, so deadlock occurs.
> The attached test case includes a thread dump at the bottom.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
