Skip to content

Commit

Permalink
Circular warning logic caused $! to be set, breaking test autorun.
Browse files Browse the repository at this point in the history
  • Loading branch information
headius committed Apr 15, 2015
1 parent f2d2fa8 commit 088b8d4
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 28 deletions.
19 changes: 14 additions & 5 deletions core/src/main/java/org/jruby/runtime/ThreadContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.jruby.internal.runtime.methods.DynamicMethod;
import org.jruby.lexer.yacc.ISourcePosition;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.backtrace.BacktraceData;
import org.jruby.runtime.backtrace.BacktraceElement;
import org.jruby.runtime.backtrace.RubyStackTraceElement;
import org.jruby.runtime.backtrace.TraceType;
Expand Down Expand Up @@ -635,11 +636,18 @@ public IRubyObject getConstant(String internedName) {
return getCurrentStaticScope().getConstant(internedName);
}

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 level
Expand All @@ -656,7 +664,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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.jruby.runtime.backtrace;

import org.jruby.Ruby;
import org.jruby.compiler.JITCompiler;
import org.jruby.util.JavaNameMangler;

import java.io.Serializable;
Expand Down Expand Up @@ -37,12 +36,12 @@ public BacktraceData(StackTraceElement[] javaTrace, BacktraceElement[] rubyTrace

public RubyStackTraceElement[] getBacktrace(Ruby runtime) {
if (backtraceElements == null) {
backtraceElements = transformBacktrace(runtime.getBoundMethods());
backtraceElements = constructBacktrace(runtime.getBoundMethods());
}
return backtraceElements;
}

private RubyStackTraceElement[] transformBacktrace(Map<String, Map<String, String>> boundMethods) {
private RubyStackTraceElement[] constructBacktrace(Map<String, Map<String, String>> boundMethods) {
List<RubyStackTraceElement> trace = new ArrayList<RubyStackTraceElement>(javaTrace.length);

// used for duplicating the previous Ruby frame when masking native calls
Expand Down
63 changes: 49 additions & 14 deletions core/src/main/java/org/jruby/runtime/backtrace/TraceType.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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);
}
},

/**
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand All @@ -341,7 +364,7 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
}
first = false;
}

buffer.append(" ");

// method name
Expand All @@ -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) {
Expand Down
11 changes: 5 additions & 6 deletions core/src/main/java/org/jruby/runtime/load/LoadService.java
Original file line number Diff line number Diff line change
Expand Up @@ -461,13 +461,12 @@ private void unlock(String requireName) {
}

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());
}

/**
Expand Down

0 comments on commit 088b8d4

Please sign in to comment.