Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: ba9f31d01431^
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: fc067824821a
Choose a head ref
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Oct 5, 2015

  1. use ReentrantLock for RubyModule$Autoload synchronization

    use some retries before giving up.
    
    Sponsored by Lookout Inc.
    mkristian committed Oct 5, 2015
    Copy the full SHA
    ba9f31d View commit details
  2. improve RequireLock to better match its semantic

    this eventually fixes #3341 in the sense that both testcases
    do not produce deadlocks anymore.
    
    Sponsored by Lookout Inc.
    mkristian committed Oct 5, 2015
    Copy the full SHA
    fc06782 View commit details
Showing with 27 additions and 13 deletions.
  1. +26 −10 core/src/main/java/org/jruby/RubyModule.java
  2. +1 −3 core/src/main/java/org/jruby/runtime/load/LoadService.java
36 changes: 26 additions & 10 deletions core/src/main/java/org/jruby/RubyModule.java
Original file line number Diff line number Diff line change
@@ -111,7 +111,10 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import static org.jruby.anno.FrameField.*;
import static org.jruby.runtime.Visibility.*;
@@ -4279,7 +4282,7 @@ private static final class Autoload {
// A ThreadContext which is executing autoload.
private volatile ThreadContext ctx;
// The lock for test-and-set the ctx.
private final Object ctxLock = new Object();
private final Lock ctxLock = new ReentrantLock();
// An object defined for the constant while autoloading.
private volatile IRubyObject value;
// A method which actually requires a defined feature.
@@ -4294,28 +4297,41 @@ private static final class Autoload {
// Returns an object for the constant if the caller is the autoloading thread.
// Otherwise, try to start autoloading and returns the defined object by autoload.
IRubyObject getConstant(ThreadContext ctx) {
synchronized (ctxLock) {
if (this.ctx == null) {
this.ctx = ctx;
} else if (isSelf(ctx)) {
return getValue();
for(int i = 10; i > 0; i--) {
try {
if (ctxLock.tryLock(10, TimeUnit.MILLISECONDS)) {

if (this.ctx == null) {
this.ctx = ctx;
} else if (isSelf(ctx)) {
return getValue();
}
// This method needs to be synchronized for removing Autoload
// from autoloadMap when it's loaded.
getLoadMethod().load(ctx.runtime);
return getValue();
}
} catch (InterruptedException e) {
} finally {
ctxLock.unlock();
}
// This method needs to be synchronized for removing Autoload
// from autoloadMap when it's loaded.
getLoadMethod().load(ctx.runtime);
}
return getValue();
}

// Update an object for the constant if the caller is the autoloading thread.
boolean setConstant(ThreadContext ctx, IRubyObject newValue) {
synchronized(ctxLock) {
try{
ctxLock.lock();
boolean isSelf = isSelf(ctx);

if (isSelf) value = newValue;

return isSelf;
}
finally {
ctxLock.unlock();
}
}

// Returns an object for the constant defined by autoload.
4 changes: 1 addition & 3 deletions core/src/main/java/org/jruby/runtime/load/LoadService.java
Original file line number Diff line number Diff line change
@@ -459,9 +459,7 @@ private boolean lock(String requireName) {

if (lock.isHeldByCurrentThread()) return false;

lock.lock();

return true;
return lock.tryLock();
}

/**