Add test for regex cache retention. #2078.

jruby · Nov 23, 2014 · a23bde8 · a23bde8
1 parent 9bafbf9
commit a23bde8
Showing 4 changed files with 64 additions and 3 deletions.
diff --git a/core/src/main/java/org/jruby/RubyRegexp.java b/core/src/main/java/org/jruby/RubyRegexp.java
@@ -138,9 +138,9 @@ public Encoding getMarshalEncoding() {
         return getEncoding();
     }
     // FIXME: Maybe these should not be static?
-    private static final WeakValuedMap<ByteList, Regex> patternCache = new WeakValuedMap();
-    private static final WeakValuedMap<ByteList, Regex> quotedPatternCache = new WeakValuedMap();
-    private static final WeakValuedMap<ByteList, Regex> preprocessedPatternCache = new WeakValuedMap();
+    static final WeakValuedMap<ByteList, Regex> patternCache = new WeakValuedMap();
+    static final WeakValuedMap<ByteList, Regex> quotedPatternCache = new WeakValuedMap();
+    static final WeakValuedMap<ByteList, Regex> preprocessedPatternCache = new WeakValuedMap();
 
     private static Regex makeRegexp(Ruby runtime, ByteList bytes, RegexpOptions options, Encoding enc) {
         try {

diff --git a/core/src/main/java/org/jruby/util/collections/WeakValuedMap.java b/core/src/main/java/org/jruby/util/collections/WeakValuedMap.java
@@ -57,6 +57,21 @@ public Value get(Key key) {
         return reference.get();
     }
 
+    public void clear() {
+        cleanReferences();
+        references.clear();
+    }
+
+    public int size() {
+        cleanReferences();
+        return references.size();
+    }
+
+    /**
+     * Construct the backing store map for this WeakValuedMap. It should be capable of safe concurrent read and write.
+     *
+     * @return the backing store map
+     */
     protected Map<Key, KeyedReference<Key, Value>> newMap() {
         return new ConcurrentHashMap();
     }

diff --git a/core/src/test/java/org/jruby/TestRegexpCache.java b/core/src/test/java/org/jruby/TestRegexpCache.java
@@ -0,0 +1,44 @@
+package org.jruby;
+
+import org.joni.Regex;
+import org.jruby.test.TestRubyBase;
+import org.jruby.util.ByteList;
+import org.jruby.util.RegexpOptions;
+
+public class TestRegexpCache extends TestRubyBase {
+    // GH-2078
+    private static final ByteList GH2078_TEST_BYTELIST = ByteList.create("GH2078");
+    public void testCacheRetention() {
+        RubyRegexp.quotedPatternCache.clear();
+        Regex regex = RubyRegexp.getQuotedRegexpFromCache(runtime, GH2078_TEST_BYTELIST, GH2078_TEST_BYTELIST.getEncoding(), RegexpOptions.NULL_OPTIONS);
+
+        // Should only have one entry
+        assertEquals(1, RubyRegexp.quotedPatternCache.size());
+
+        // Should be the same object if cached
+        assertSame(regex, RubyRegexp.getQuotedRegexpFromCache(runtime, GH2078_TEST_BYTELIST, GH2078_TEST_BYTELIST.getEncoding(), RegexpOptions.NULL_OPTIONS));
+
+        // clear reference and attempt to trigger GC
+        regex = null;
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+        Thread.yield();
+        System.gc();
+
+        // Should be no retained references once cleaned
+        assertEquals(0, RubyRegexp.quotedPatternCache.size());
+    }
+}
diff --git a/core/src/test/java/org/jruby/test/MainTestSuite.java b/core/src/test/java/org/jruby/test/MainTestSuite.java
@@ -36,6 +36,7 @@
 import junit.framework.Test;
 import junit.framework.TestSuite;
 
+import org.jruby.TestRegexpCache;
 import org.jruby.ext.posix.JavaFileStatTest;
 import org.jruby.javasupport.TestJava;
 import org.jruby.javasupport.TestJavaClass;
@@ -94,6 +95,7 @@ public static Test suite() throws Throwable {
         suite.addTestSuite(ParameterizedWriterTest.class);
         suite.addTestSuite(TestRubyRational.class);
         suite.addTestSuite(TestRecursiveCheck.class);
+        suite.addTestSuite(TestRegexpCache.class);
         return suite;
     }
 }