Skip to content

Commit

Permalink
Use thread-local state for all recursive checks.
Browse files Browse the repository at this point in the history
Backport from 46c59ad. Partial fix for jruby#162.
  • Loading branch information
headius authored and João Duarte committed Sep 4, 2012
1 parent 26e08ba commit 3aab61b
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
11 changes: 11 additions & 0 deletions spec/regression/recursive_check_thread_safety_spec.rb
@@ -0,0 +1,11 @@
require 'rspec'

describe 'Joining an array that contains arrays' do
it 'should work across multiple threads without error' do
a = [(1..100).to_a]
lambda do
result = (1..100).map { Thread.new { 10.times { a.join }; :ok } }.map(&:value)
result.should == [:ok] * 100
end.should_not raise_exception
end
end
19 changes: 10 additions & 9 deletions src/org/jruby/Ruby.java
Expand Up @@ -1377,7 +1377,8 @@ private void initExceptions() {
keyError = defineClassIfAllowed("KeyError", indexError);

mathDomainError = defineClassUnder("DomainError", argumentError, argumentError.getAllocator(), mathModule);
recursiveKey = newSymbol("__recursive_key__");
recursiveKey.set(newSymbol("__recursive_key__"));
inRecursiveListOperation.set(false);
}

initErrno();
Expand Down Expand Up @@ -3659,7 +3660,7 @@ private IRubyObject execRecursiveInternal(RecursiveFunction func, IRubyObject ob
ExecRecursiveParams p = new ExecRecursiveParams();
p.list = recursiveListAccess();
p.objid = obj.id();
boolean outermost = outer && !recursiveCheck(p.list, recursiveKey, null);
boolean outermost = outer && !recursiveCheck(p.list, recursiveKey.get(), null);
if(recursiveCheck(p.list, p.objid, pairid)) {
if(outer && !outermost) {
throw new RecursiveError(p.list);
Expand All @@ -3672,7 +3673,7 @@ private IRubyObject execRecursiveInternal(RecursiveFunction func, IRubyObject ob
p.pairid = pairid;

if(outermost) {
recursivePush(p.list, recursiveKey, null);
recursivePush(p.list, recursiveKey.get(), null);
try {
result = execRecursiveI(p);
} catch(RecursiveError e) {
Expand All @@ -3682,7 +3683,7 @@ private IRubyObject execRecursiveInternal(RecursiveFunction func, IRubyObject ob
result = p.list;
}
}
recursivePop(p.list, recursiveKey, null);
recursivePop(p.list, recursiveKey.get(), null);
if(result == p.list) {
result = func.call(obj, true);
}
Expand Down Expand Up @@ -3712,7 +3713,7 @@ private IRubyObject execRecursiveInternal(RecursiveFunction func, IRubyObject ob
* @return
*/
public IRubyObject execRecursive(RecursiveFunction func, IRubyObject obj) {
if (!inRecursiveListOperation) {
if (!inRecursiveListOperation.get()) {
throw newThreadError("BUG: execRecursive called outside recursiveListOperation");
}
return execRecursiveInternal(func, obj, null, false);
Expand Down Expand Up @@ -3751,14 +3752,14 @@ public IRubyObject execRecursiveOuter(RecursiveFunction func, IRubyObject obj) {
*/
public <T extends IRubyObject> T recursiveListOperation(Callable<T> body) {
try {
inRecursiveListOperation = true;
inRecursiveListOperation.set(true);
return body.call();
} catch (Exception e) {
UnsafeFactory.getUnsafe().throwException(e);
return null; // not reached
} finally {
recursiveListClear();
inRecursiveListOperation = false;
inRecursiveListOperation.set(false);
}
}

Expand Down Expand Up @@ -4280,6 +4281,6 @@ public CoverageData getCoverageData() {

// structures and such for recursive operations
private ThreadLocal<Map<String, RubyHash>> recursive = new ThreadLocal<Map<String, RubyHash>>();
private RubySymbol recursiveKey;
private boolean inRecursiveListOperation;
private ThreadLocal<RubySymbol> recursiveKey = new ThreadLocal<RubySymbol>();
private ThreadLocal<Boolean> inRecursiveListOperation = new ThreadLocal<Boolean>();
}

0 comments on commit 3aab61b

Please sign in to comment.