You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2017/05/25 23:38:16 UTC
groovy git commit: GROOVY-7535: Groovy category throwing
MissingMethodException and MissingPropertyException when using multiple
threads (closes #540)
Repository: groovy
Updated Branches:
refs/heads/master 7e4caea28 -> eef1304c6
GROOVY-7535: Groovy category throwing MissingMethodException and MissingPropertyException when using multiple threads (closes #540)
Changes based on a patch supplied by Jochen Kemnade.
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/eef1304c
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/eef1304c
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/eef1304c
Branch: refs/heads/master
Commit: eef1304c6038682c7fe589aaa3b2eff4fe459652
Parents: 7e4caea
Author: John Wagenleitner <jw...@apache.org>
Authored: Sat May 13 18:51:47 2017 -0700
Committer: John Wagenleitner <jw...@apache.org>
Committed: Thu May 25 16:31:18 2017 -0700
----------------------------------------------------------------------
.../groovy/runtime/GroovyCategorySupport.java | 42 +++++++--
.../runtime/GroovyCategoryStressTest.groovy | 96 ++++++++++++++++++++
2 files changed, 131 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/eef1304c/src/main/org/codehaus/groovy/runtime/GroovyCategorySupport.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/GroovyCategorySupport.java b/src/main/org/codehaus/groovy/runtime/GroovyCategorySupport.java
index 4c36935..bb7b7ae 100644
--- a/src/main/org/codehaus/groovy/runtime/GroovyCategorySupport.java
+++ b/src/main/org/codehaus/groovy/runtime/GroovyCategorySupport.java
@@ -40,7 +40,6 @@ import java.util.concurrent.atomic.AtomicInteger;
public class GroovyCategorySupport {
private static int categoriesInUse = 0;
- private static final AtomicInteger atomicCategoryUsageCounter = new AtomicInteger();
public static class CategoryMethodList extends ArrayList<CategoryMethod> {
public final int level;
@@ -66,14 +65,19 @@ public class GroovyCategorySupport {
}
public static class ThreadCategoryInfo extends HashMap<String, CategoryMethodList>{
+
+ private static final Object LOCK = new Object();
+
int level;
private Map<String, String> propertyGetterMap;
private Map<String, String> propertySetterMap;
private void newScope () {
- categoriesInUse = atomicCategoryUsageCounter.incrementAndGet();
- DefaultMetaClassInfo.setCategoryUsed(true);
+ synchronized (LOCK) {
+ categoriesInUse++;
+ DefaultMetaClassInfo.setCategoryUsed(true);
+ }
VMPluginFactory.getPlugin().invalidateCallSites();
level++;
}
@@ -95,9 +99,12 @@ public class GroovyCategorySupport {
}
}
level--;
- categoriesInUse = atomicCategoryUsageCounter.decrementAndGet();
VMPluginFactory.getPlugin().invalidateCallSites();
- if (categoriesInUse==0) DefaultMetaClassInfo.setCategoryUsed(false);
+ synchronized (LOCK) {
+ if (--categoriesInUse == 0) {
+ DefaultMetaClassInfo.setCategoryUsed(false);
+ }
+ }
if (level == 0) {
THREAD_INFO.remove();
}
@@ -262,13 +269,34 @@ public class GroovyCategorySupport {
}
public static boolean hasCategoryInCurrentThread() {
- if (categoriesInUse == 0) return false;
+ /*
+ * Synchronization is avoided here for performance reasons since
+ * this method is called frequently from callsite locations. For
+ * a typical case when no Categories are in use the initialized
+ * value of 0 will be correctly read. For cases where multiple
+ * Threads are using Categories it is possible that a stale
+ * non-zero value may be read but in that case the ThreadLocal
+ * check will produce the correct result. When the current Thread
+ * is using Categories, it would have incremented the counter
+ * so whatever version of the value it observes here should be
+ * non-zero and good enough for the purposes of this quick exit
+ * check.
+ */
+ if (categoriesInUse == 0) {
+ return false;
+ }
ThreadCategoryInfo infoNullable = THREAD_INFO.getInfoNullable();
return infoNullable != null && infoNullable.level != 0;
}
+ /**
+ * @deprecated use {@link #hasCategoryInCurrentThread()}
+ */
+ @Deprecated
public static boolean hasCategoryInAnyThread() {
- return atomicCategoryUsageCounter.get() != 0;
+ synchronized (ThreadCategoryInfo.LOCK) {
+ return categoriesInUse != 0;
+ }
}
/**
http://git-wip-us.apache.org/repos/asf/groovy/blob/eef1304c/subprojects/stress/src/test/groovy/org/codehaus/groovy/runtime/GroovyCategoryStressTest.groovy
----------------------------------------------------------------------
diff --git a/subprojects/stress/src/test/groovy/org/codehaus/groovy/runtime/GroovyCategoryStressTest.groovy b/subprojects/stress/src/test/groovy/org/codehaus/groovy/runtime/GroovyCategoryStressTest.groovy
new file mode 100644
index 0000000..e981a7a
--- /dev/null
+++ b/subprojects/stress/src/test/groovy/org/codehaus/groovy/runtime/GroovyCategoryStressTest.groovy
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.runtime
+
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.ConcurrentMap
+import java.util.concurrent.CyclicBarrier
+import java.util.concurrent.TimeUnit
+
+/**
+ * Stress test the concurrent use of Groovy Categories.
+ *
+ */
+class GroovyCategoryStressTest extends GroovyTestCase {
+
+ private static final int THREAD_COUNT = 4
+ private static final int TEST_LOOPS = 500
+
+ final CyclicBarrier barrier = new CyclicBarrier(THREAD_COUNT + 1)
+ final ConcurrentMap<Thread, Throwable> failures = new ConcurrentHashMap<Thread, Throwable>()
+ final TestThreadExceptionHandler exceptionHandler = new TestThreadExceptionHandler()
+
+ /**
+ * GROOVY-7535
+ *
+ * Detecting failures is dependent on thread timing so it requires
+ * looping many times in order to try to replicate the problem.
+ */
+ void testWithMultipleThreads() {
+ TEST_LOOPS.times {
+ if (((it + 1) % 50) == 0) {
+ println "Iteration ${it + 1} of ${TEST_LOOPS}"
+ }
+ performTest()
+ if (!failures.isEmpty()) {
+ println 'Failures: ' + failures.size()
+ Throwable t = failures.values().first()
+ throw t
+ }
+ }
+ }
+
+ private void performTest() {
+ def test = {
+ barrier.await()
+ TEST_LOOPS.times {
+ use(MyTestCategory) {
+ assert 'foo'.testGroovy7535(' bar') == 'foo bar'
+ assert 'foo'.testGroovy7535(' bar') == 'foo bar'
+ assert 'foo'.testGroovy7535(' bar') == 'foo bar'
+ assert 'foo'.testGroovy7535(' bar') == 'foo bar'
+ }
+ }
+ barrier.await()
+ }
+ THREAD_COUNT.times {
+ Thread th = new Thread(test)
+ th.setDaemon(true)
+ th.setUncaughtExceptionHandler(exceptionHandler)
+ th.start()
+ }
+ barrier.await() // start all threads
+ barrier.await(1L, TimeUnit.MINUTES)
+ }
+
+ private static class MyTestCategory {
+ static String testGroovy7535(String self, String foo) {
+ return self + foo
+ }
+ }
+
+ private class TestThreadExceptionHandler implements Thread.UncaughtExceptionHandler {
+ @Override
+ void uncaughtException(Thread t, Throwable e) {
+ failures.put(t, e)
+ barrier.await()
+ }
+ }
+
+}