Skip to content

Commit a23bde8

Browse files
committed
Add test for regex cache retention. #2078.
1 parent 9bafbf9 commit a23bde8

File tree

4 files changed

+64
-3
lines changed

4 files changed

+64
-3
lines changed

core/src/main/java/org/jruby/RubyRegexp.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ public Encoding getMarshalEncoding() {
138138
return getEncoding();
139139
}
140140
// FIXME: Maybe these should not be static?
141-
private static final WeakValuedMap<ByteList, Regex> patternCache = new WeakValuedMap();
142-
private static final WeakValuedMap<ByteList, Regex> quotedPatternCache = new WeakValuedMap();
143-
private static final WeakValuedMap<ByteList, Regex> preprocessedPatternCache = new WeakValuedMap();
141+
static final WeakValuedMap<ByteList, Regex> patternCache = new WeakValuedMap();
142+
static final WeakValuedMap<ByteList, Regex> quotedPatternCache = new WeakValuedMap();
143+
static final WeakValuedMap<ByteList, Regex> preprocessedPatternCache = new WeakValuedMap();
144144

145145
private static Regex makeRegexp(Ruby runtime, ByteList bytes, RegexpOptions options, Encoding enc) {
146146
try {

core/src/main/java/org/jruby/util/collections/WeakValuedMap.java

+15
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,21 @@ public Value get(Key key) {
5757
return reference.get();
5858
}
5959

60+
public void clear() {
61+
cleanReferences();
62+
references.clear();
63+
}
64+
65+
public int size() {
66+
cleanReferences();
67+
return references.size();
68+
}
69+
70+
/**
71+
* Construct the backing store map for this WeakValuedMap. It should be capable of safe concurrent read and write.
72+
*
73+
* @return the backing store map
74+
*/
6075
protected Map<Key, KeyedReference<Key, Value>> newMap() {
6176
return new ConcurrentHashMap();
6277
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package org.jruby;
2+
3+
import org.joni.Regex;
4+
import org.jruby.test.TestRubyBase;
5+
import org.jruby.util.ByteList;
6+
import org.jruby.util.RegexpOptions;
7+
8+
public class TestRegexpCache extends TestRubyBase {
9+
// GH-2078
10+
private static final ByteList GH2078_TEST_BYTELIST = ByteList.create("GH2078");
11+
public void testCacheRetention() {
12+
RubyRegexp.quotedPatternCache.clear();
13+
Regex regex = RubyRegexp.getQuotedRegexpFromCache(runtime, GH2078_TEST_BYTELIST, GH2078_TEST_BYTELIST.getEncoding(), RegexpOptions.NULL_OPTIONS);
14+
15+
// Should only have one entry
16+
assertEquals(1, RubyRegexp.quotedPatternCache.size());
17+
18+
// Should be the same object if cached
19+
assertSame(regex, RubyRegexp.getQuotedRegexpFromCache(runtime, GH2078_TEST_BYTELIST, GH2078_TEST_BYTELIST.getEncoding(), RegexpOptions.NULL_OPTIONS));
20+
21+
// clear reference and attempt to trigger GC
22+
regex = null;
23+
System.gc();
24+
Thread.yield();
25+
System.gc();
26+
Thread.yield();
27+
System.gc();
28+
Thread.yield();
29+
System.gc();
30+
Thread.yield();
31+
System.gc();
32+
Thread.yield();
33+
System.gc();
34+
Thread.yield();
35+
System.gc();
36+
Thread.yield();
37+
System.gc();
38+
Thread.yield();
39+
System.gc();
40+
41+
// Should be no retained references once cleaned
42+
assertEquals(0, RubyRegexp.quotedPatternCache.size());
43+
}
44+
}

core/src/test/java/org/jruby/test/MainTestSuite.java

+2
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import junit.framework.Test;
3737
import junit.framework.TestSuite;
3838

39+
import org.jruby.TestRegexpCache;
3940
import org.jruby.ext.posix.JavaFileStatTest;
4041
import org.jruby.javasupport.TestJava;
4142
import org.jruby.javasupport.TestJavaClass;
@@ -94,6 +95,7 @@ public static Test suite() throws Throwable {
9495
suite.addTestSuite(ParameterizedWriterTest.class);
9596
suite.addTestSuite(TestRubyRational.class);
9697
suite.addTestSuite(TestRecursiveCheck.class);
98+
suite.addTestSuite(TestRegexpCache.class);
9799
return suite;
98100
}
99101
}

0 commit comments

Comments
 (0)