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 #162.
  • Loading branch information
headius committed Sep 5, 2012
1 parent 102f85a commit 2f73450
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
Original file line number Original file line Diff line number Diff line change
@@ -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
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1377,7 +1377,8 @@ private void initExceptions() {
keyError = defineClassIfAllowed("KeyError", indexError); keyError = defineClassIfAllowed("KeyError", indexError);


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


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


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


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


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

0 comments on commit 2f73450

Please sign in to comment.