You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2015/12/09 18:33:00 UTC
calcite git commit: [CALCITE-1009] SelfPopulatingList is not
thread-safe
Repository: calcite
Updated Branches:
refs/heads/master de3880298 -> a850c41e2
[CALCITE-1009] SelfPopulatingList is not thread-safe
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/a850c41e
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/a850c41e
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/a850c41e
Branch: refs/heads/master
Commit: a850c41e2da262234475ec56383457475e9fabee
Parents: de38802
Author: Julian Hyde <jh...@apache.org>
Authored: Tue Dec 8 21:40:01 2015 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Tue Dec 8 23:02:10 2015 -0800
----------------------------------------------------------------------
.../java/org/apache/calcite/rex/RexSlot.java | 11 +++--
.../org/apache/calcite/rex/RexExecutorTest.java | 46 ++++++++++++++++++++
2 files changed, 54 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/a850c41e/core/src/main/java/org/apache/calcite/rex/RexSlot.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rex/RexSlot.java b/core/src/main/java/org/apache/calcite/rex/RexSlot.java
index 5421f12..d56db42 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSlot.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSlot.java
@@ -89,9 +89,14 @@ public abstract class RexSlot extends RexVariable {
if (index < 0) {
throw new IllegalArgumentException();
}
- addAll(
- fromTo(
- prefix, size(), Math.max(index + 1, size() * 2)));
+ // Double-checked locking, but safe because CopyOnWriteArrayList.array
+ // is marked volatile, and size() uses array.length.
+ synchronized (this) {
+ final int size = size();
+ if (index >= size) {
+ addAll(fromTo(prefix, size, Math.max(index + 1, size * 2)));
+ }
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/calcite/blob/a850c41e/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java b/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
index d4780a6..72a36e2 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
@@ -41,9 +41,11 @@ import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.List;
+import java.util.Random;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@@ -211,6 +213,50 @@ public class RexExecutorTest {
});
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-1009">[CALCITE-1009]
+ * SelfPopulatingList is not thread-safe</a>. */
+ @Test public void testSelfPopulatingList() {
+ final List<Thread> threads = new ArrayList<>();
+ //noinspection MismatchedQueryAndUpdateOfCollection
+ final List<String> list = new RexSlot.SelfPopulatingList("$", 1);
+ final Random random = new Random();
+ for (int i = 0; i < 10; i++) {
+ threads.add(
+ new Thread() {
+ public void run() {
+ for (int j = 0; j < 1000; j++) {
+ // Random numbers between 0 and ~1m, smaller values more common
+ final int index = random.nextInt(1234567)
+ >> random.nextInt(16) >> random.nextInt(16);
+ list.get(index);
+ }
+ }
+ });
+ }
+ for (Thread runnable : threads) {
+ runnable.start();
+ }
+ for (Thread runnable : threads) {
+ try {
+ runnable.join();
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ }
+ final int size = list.size();
+ for (int i = 0; i < size; i++) {
+ assertThat(list.get(i), is("$" + i));
+ }
+ }
+
+ @Test public void testSelfPopulatingList30() {
+ //noinspection MismatchedQueryAndUpdateOfCollection
+ final List<String> list = new RexSlot.SelfPopulatingList("$", 30);
+ final String s = list.get(30);
+ assertThat(s, is("$30"));
+ }
+
/** Callback for {@link #check}. Test code will typically use {@code builder}
* to create some expressions, call
* {@link org.apache.calcite.rex.RexExecutorImpl#reduce} to evaluate them into