Skip to content

Commit

Permalink
Transplant recent LoadService fixes from master.
Browse files Browse the repository at this point in the history 
* Fix autoload in the presence of canonicalized paths
* Fix require locking in the presence of canonicalized paths
* Fix circular require warning setting $!
@headius
headius committed Apr 20, 2015
1 parent c384fd0 commit 1735752
Showing 3 changed files with 134 additions and 118 deletions.
24 changes: 15 additions & 9 deletions core/src/main/java/org/jruby/runtime/ThreadContext.java
View file
Original file line number Diff line number Diff line change
@@ -48,14 +48,12 @@
import org.jruby.ext.fiber.ThreadFiber;
import org.jruby.internal.runtime.methods.DynamicMethod;
import org.jruby.lexer.yacc.ISourcePosition;
import org.jruby.ext.fiber.ThreadFiber;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.backtrace.BacktraceData;
import org.jruby.runtime.backtrace.TraceType;
import org.jruby.runtime.backtrace.TraceType.Gather;
import org.jruby.runtime.backtrace.BacktraceElement;
import org.jruby.runtime.backtrace.RubyStackTraceElement;
import org.jruby.runtime.backtrace.TraceType;
import org.jruby.runtime.backtrace.TraceType.Gather;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.profile.ProfileCollection;
import org.jruby.runtime.scope.ManyVarsDynamicScope;
@@ -766,12 +764,19 @@ public IRubyObject setConstantInObject(String internedName, IRubyObject result)

return result;
}

private static void addBackTraceElement(Ruby runtime, RubyArray backtrace, RubyStackTraceElement element) {
RubyString str = RubyString.newString(runtime, element.mriStyleString());
backtrace.append(str);

/**
* Render the current backtrace as a string to the given StringBuilder. This will honor the currently-configured
* backtrace format and content.
*
* @param sb the StringBuilder to which to render the backtrace
*/
public void renderCurrentBacktrace(StringBuilder sb) {
TraceType traceType = runtime.getInstanceConfig().getTraceType();
BacktraceData backtraceData = traceType.getBacktrace(this, false);
traceType.getFormat().renderBacktrace(backtraceData.getBacktrace(runtime), sb, false);
}

/**
* Create an Array with backtrace information for Kernel#caller
* @param runtime
@@ -799,7 +804,8 @@ public IRubyObject createCallerBacktrace(int level, Integer length, StackTraceEl
RubyArray newTrace = runtime.newArray(trace.length);

for (int i = level; i - level < trace.length; i++) {
addBackTraceElement(runtime, newTrace, trace[i - level]);
RubyString str = RubyString.newString(runtime, trace[i - level].mriStyleString());
newTrace.append(str);
}

if (RubyInstanceConfig.LOG_CALLERS) TraceType.dumpCaller(newTrace);
83 changes: 59 additions & 24 deletions core/src/main/java/org/jruby/runtime/backtrace/TraceType.java
View file
Original file line number Diff line number Diff line change
@@ -28,6 +28,14 @@ public TraceType(Gather gather, Format format) {
this.format = format;
}

public Gather getGather() {
return gather;
}

public Format getFormat() {
return format;
}

/**
* Get a normal Ruby backtrace, using the current Gather type.
*
@@ -60,20 +68,20 @@ public static void logBacktrace(RubyStackTraceElement[] trace) {
LOG.info(" " + element.getFileName() + ":" + element.getLineNumber() + " in " + element.getMethodName());
}
}

public static void dumpException(RubyException exception) {
LOG.info("Exception raised: {} : {}", exception.getMetaClass(), exception);
}

public static void dumpBacktrace(RubyException exception) {
Ruby runtime = exception.getRuntime();
System.err.println("Backtrace generated:\n" + Format.JRUBY.printBacktrace(exception, runtime.getPosix().isatty(FileDescriptor.err)));
}

public static void dumpCaller(RubyArray trace) {
LOG.info("Caller backtrace generated:\n" + trace);
}

public static void dumpCaller(RubyStackTraceElement[] trace) {
LOG.info("Caller backtrace generated:\n" + Arrays.toString(trace));
}
@@ -86,13 +94,13 @@ public static TraceType traceTypeFor(String style) {
if (style.equalsIgnoreCase("raw")) return new TraceType(Gather.RAW, Format.JRUBY);
else if (style.equalsIgnoreCase("ruby_framed")) return new TraceType(Gather.NORMAL, Format.JRUBY);
else if (style.equalsIgnoreCase("normal")) return new TraceType(Gather.NORMAL, Format.JRUBY);
// deprecated, just uses jruby format now
// deprecated, just uses jruby format now
else if (style.equalsIgnoreCase("rubinius")) return new TraceType(Gather.NORMAL, Format.JRUBY);
else if (style.equalsIgnoreCase("full")) return new TraceType(Gather.FULL, Format.JRUBY);
else if (style.equalsIgnoreCase("mri")) return new TraceType(Gather.NORMAL, Format.MRI);
else return new TraceType(Gather.NORMAL, Format.JRUBY);
}

public enum Gather {
/**
* Full raw backtraces with all Java frames included.
@@ -113,7 +121,7 @@ public BacktraceData getBacktraceData(ThreadContext context, StackTraceElement[]
*/
FULL {
public BacktraceData getBacktraceData(ThreadContext context, StackTraceElement[] javaTrace, boolean nativeException) {
return new BacktraceData(
return new BacktraceData(
javaTrace,
context.createBacktrace2(0, nativeException),
true,
@@ -183,7 +191,7 @@ public BacktraceData getBacktraceData(ThreadContext context, boolean nativeExcep
/**
* Gather backtrace data for an integrated trace if the current gather type is "NORMAL", otherwise use the
* current gather type.
*
*
* @param context
* @param javaTrace
* @return
@@ -194,7 +202,7 @@ public BacktraceData getIntegratedBacktraceData(ThreadContext context, StackTrac
if (useGather == NORMAL) {
useGather = INTEGRATED;
}

BacktraceData data = useGather.getBacktraceData(context, javaTrace, false);

context.runtime.incrementBacktraceCount();
@@ -205,7 +213,7 @@ public BacktraceData getIntegratedBacktraceData(ThreadContext context, StackTrac

public abstract BacktraceData getBacktraceData(ThreadContext context, StackTraceElement[] javaTrace, boolean nativeException);
}

public enum Format {
/**
* Formatting like C Ruby
@@ -214,6 +222,10 @@ public enum Format {
public String printBacktrace(RubyException exception, boolean console) {
return printBacktraceMRI(exception, console);
}

public void renderBacktrace(RubyStackTraceElement[] elts, StringBuilder buffer, boolean color) {
renderBacktraceMRI(elts, buffer, color);
}
},

/**
@@ -223,9 +235,14 @@ public String printBacktrace(RubyException exception, boolean console) {
public String printBacktrace(RubyException exception, boolean console) {
return printBacktraceJRuby(exception, console);
}

public void renderBacktrace(RubyStackTraceElement[] elts, StringBuilder buffer, boolean color) {
renderBacktraceJRuby(elts, buffer, color);
}
};

public abstract String printBacktrace(RubyException exception, boolean console);
public abstract void renderBacktrace(RubyStackTraceElement[] elts, StringBuilder buffer, boolean color);
}

protected static String printBacktraceMRI(RubyException exception, boolean console) {
@@ -304,16 +321,9 @@ protected static String printBacktraceMRI(RubyException exception, boolean conso

protected static String printBacktraceJRuby(RubyException exception, boolean console) {
Ruby runtime = exception.getRuntime();
RubyStackTraceElement[] frames = exception.getBacktraceElements();
if (frames == null) frames = new RubyStackTraceElement[0];

// find longest method name
int longestMethod = 0;
for (RubyStackTraceElement frame : frames) {
longestMethod = Math.max(longestMethod, frame.getMethodName().length());
}

StringBuilder buffer = new StringBuilder();
boolean color = console && runtime.getInstanceConfig().getBacktraceColor();

// exception line
String message = exception.message(runtime.getCurrentContext()).toString();
@@ -325,8 +335,21 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
.append(": ")
.append(message)
.append('\n');

boolean color = console && runtime.getInstanceConfig().getBacktraceColor();

RubyStackTraceElement[] frames = exception.getBacktraceElements();
if (frames == null) frames = RubyStackTraceElement.EMPTY_ARRAY;
renderBacktraceJRuby(frames, buffer, color);


return buffer.toString();
}

private static void renderBacktraceJRuby(RubyStackTraceElement[] frames, StringBuilder buffer, boolean color) {
// find longest method name
int longestMethod = 0;
for (RubyStackTraceElement frame : frames) {
longestMethod = Math.max(longestMethod, frame.getMethodName().length());
}

// backtrace lines
boolean first = true;
@@ -341,7 +364,7 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
}
first = false;
}

buffer.append(" ");

// method name
@@ -355,16 +378,28 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
.append(frame.getFileName())
.append(':')
.append(frame.getLineNumber());

if (color) {
buffer.append(CLEAR_COLOR);
}

buffer
.append('\n');
}
}

return buffer.toString();
private static void renderBacktraceMRI(RubyStackTraceElement[] trace, StringBuilder buffer, boolean color) {
for (int i = 0; i < trace.length; i++) {
RubyStackTraceElement element = trace[i];

buffer
.append(element.getFileName())
.append(':')
.append(element.getLineNumber())
.append(":in `")
.append(element.getMethodName())
.append("'\n");
}
}

public static IRubyObject generateMRIBacktrace(Ruby runtime, RubyStackTraceElement[] trace) {
145 changes: 60 additions & 85 deletions core/src/main/java/org/jruby/runtime/load/LoadService.java
View file
Original file line number Diff line number Diff line change
@@ -422,121 +422,118 @@ private enum RequireState {
LOADED, ALREADY_LOADED, CIRCULAR
};

private RequireState requireCommon(String requireName, boolean circularRequireWarning) {
// check for requiredName without extension.
if (featureAlreadyLoaded(requireName)) {
private RequireState requireCommon(String file, boolean circularRequireWarning) {
checkEmptyLoad(file);

// check with short name
if (featureAlreadyLoaded(file)) {
return RequireState.ALREADY_LOADED;
}

SearchState state = findFileForLoad(file);

if (state.library == null) {
throw runtime.newLoadError("no such file to load -- " + state.searchFile, state.searchFile);
}

// check with long name
if (featureAlreadyLoaded(state.loadName)) {
return RequireState.ALREADY_LOADED;
}

if (!requireLocks.lock(requireName)) {
if (circularRequireWarning && runtime.isVerbose() && runtime.is1_9()) {
warnCircularRequire(requireName);
if (!requireLocks.lock(state.loadName)) {
if (circularRequireWarning && runtime.isVerbose()) {
warnCircularRequire(state.loadName);
}
return RequireState.CIRCULAR;
}

// numbers from loadTimer does not include lock waiting time.
long startTime = loadTimer.startLoad(state.loadName);

try {
if (!runtime.getProfile().allowRequire(requireName)) {
throw runtime.newLoadError("no such file to load -- " + requireName, requireName);
// check with short name again
if (featureAlreadyLoaded(file)) {
return RequireState.ALREADY_LOADED;
}

// check for requiredName again now that we're locked
if (featureAlreadyLoaded(requireName)) {
// check with long name again in case it loaded while we were locking
if (featureAlreadyLoaded(state.loadName)) {
return RequireState.ALREADY_LOADED;
}

// numbers from loadTimer does not include lock waiting time.
long startTime = loadTimer.startLoad(requireName);
try {
boolean loaded = smartLoadInternal(requireName);
return loaded ? RequireState.LOADED : RequireState.ALREADY_LOADED;
} finally {
loadTimer.endLoad(requireName, startTime);
boolean loaded = tryLoadingLibraryOrScript(runtime, state);
if (loaded) {
addLoadedFeature(file, state.loadName);
}
return loaded ? RequireState.LOADED : RequireState.ALREADY_LOADED;
} finally {
requireLocks.unlock(requireName);
loadTimer.endLoad(state.loadName, startTime);
requireLocks.unlock(state.loadName);
}
}

protected final RequireLocks requireLocks = new RequireLocks();

private class RequireLocks {
private final Map<String, ReentrantLock> pool;
private final ConcurrentHashMap<String, ReentrantLock> pool;
// global lock for require must be fair
private final ReentrantLock globalLock;

private RequireLocks() {
this.pool = new HashMap<String, ReentrantLock>();
this.pool = new ConcurrentHashMap<String, ReentrantLock>(8, 0.75f, 2);
this.globalLock = new ReentrantLock(true);
}

/**
* Get exclusive lock for the specified requireName. Acquire sync object
* for the requireName from the pool, then try to lock it. NOTE: This
* lock is not fair for now.
*
*
* @param requireName
* just a name for the lock.
* @return If the sync object already locked by current thread, it just
* returns false without getting a lock. Otherwise true.
*/
private boolean lock(String requireName) {
ReentrantLock lock;

while (true) {
synchronized (pool) {
lock = pool.get(requireName);
if (lock == null) {
if (runtime.getInstanceConfig().isGlobalRequireLock()) {
lock = globalLock;
} else {
lock = new ReentrantLock();
}
pool.put(requireName, lock);
} else if (lock.isHeldByCurrentThread()) {
return false;
}
}

lock.lock();
ReentrantLock lock = pool.get(requireName);

// repeat until locked object still in requireLocks.
synchronized (pool) {
if (pool.get(requireName) == lock) {
// the object is locked && the lock is in the pool
return true;
}
// go next try
lock.unlock();
}
if (lock == null) {
ReentrantLock newLock = new ReentrantLock();
lock = pool.putIfAbsent(requireName, newLock);
if (lock == null) lock = newLock;
}

if (lock.isHeldByCurrentThread()) return false;

lock.lock();

return true;
}

/**
* Unlock the lock for the specified requireName.
*
*
* @param requireName
* name of the lock to be unlocked.
*/
private void unlock(String requireName) {
synchronized (pool) {
ReentrantLock lock = pool.get(requireName);
if (lock != null) {
assert lock.isHeldByCurrentThread();
lock.unlock();
pool.remove(requireName);
}
ReentrantLock lock = pool.get(requireName);

if (lock != null) {
assert lock.isHeldByCurrentThread();
lock.unlock();
}
}
}

protected void warnCircularRequire(String requireName) {
StringBuilder sb = new StringBuilder();

runtime.getCurrentContext().renderCurrentBacktrace(sb);

runtime.getWarnings().warn("loading in progress, circular require considered harmful - " + requireName);
// it's a hack for c:rb_backtrace impl.
// We should introduce new method to Ruby.TraceType when rb_backtrace is widely used not only for this purpose.
RaiseException ex = new RaiseException(runtime, runtime.getRuntimeError(), null, false);
String trace = runtime.getInstanceConfig().getTraceType().printBacktrace(ex.getException(), runtime.getPosix().isatty(FileDescriptor.err));
// rb_backtrace dumps to stderr directly.
System.err.print(trace.replaceFirst("[^\n]*\n", ""));
runtime.getErr().print(sb.toString());
}

/**
@@ -548,28 +545,6 @@ public boolean smartLoad(String file) {
return require(file);
}

private boolean smartLoadInternal(String file) {
checkEmptyLoad(file);
SearchState state = findFileForLoad(file);
if (state == null) {
return false;
}
if (state.library == null) {
throw runtime.newLoadError("no such file to load -- " + state.searchFile, state.searchFile);
}

// check with long name
if (featureAlreadyLoaded(state.loadName)) {
return false;
}

boolean loaded = tryLoadingLibraryOrScript(runtime, state);
if (loaded) {
addLoadedFeature(file, state.loadName);
}
return loaded;
}

private static class LoadTimer {
public long startLoad(String file) { return 0L; }
public void endLoad(String file, long startTime) {}

0 comments on commit 1735752

Please sign in to comment.