Skip to content

Commit

Permalink
Exception.cause was NPEing (one constructor path was not initializing…
Browse files Browse the repository at this point in the history
… it).
  • Loading branch information
enebo committed Mar 30, 2018
1 parent 259b89e commit 64d4fd8
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
7 changes: 3 additions & 4 deletions core/src/main/java/org/jruby/RubyException.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public class RubyException extends RubyObject {
public static final int TRACE_MAX = RubyException.TRACE_HEAD + RubyException.TRACE_TAIL + 6;
protected BacktraceData backtraceData;
IRubyObject message;
IRubyObject cause;
IRubyObject cause = UNDEF;
private IRubyObject backtrace;
private RaiseException throwable;
private IRubyObject backtraceLocations;
Expand All @@ -84,7 +84,6 @@ public RubyException(Ruby runtime, RubyClass rubyClass, String message) {
super(runtime, rubyClass);

this.setMessage(message == null ? runtime.getNil() : runtime.newString(message));
this.cause = RubyBasicObject.UNDEF;
}

@JRubyMethod(name = "exception", optional = 1, rest = true, meta = true)
Expand Down Expand Up @@ -210,7 +209,7 @@ private void setBacktrace(IRubyObject obj) {
@JRubyMethod(omit = true)
public IRubyObject backtrace_locations(ThreadContext context) {
if (backtraceLocations != null) return backtraceLocations;

if (backtraceData == null) {
backtraceLocations = context.nil;
} else {
Expand Down Expand Up @@ -327,7 +326,7 @@ public void setCause(IRubyObject cause) {

// NOTE: can not have IRubyObject as NativeException has getCause() returning Throwable
public Object getCause() {
return cause == RubyBasicObject.UNDEF ? null : cause;
return cause.isNil() ? null : cause;

This comment has been minimized.

Copy link
@kares

kares Mar 31, 2018

Member

I do not get this - why would we rather want to 'leak' UNDEF outside for a presumable Java getCause caller ?

This comment has been minimized.

Copy link
@enebo

enebo Mar 31, 2018

Author Member

@kares doh that is a mistake. I originally did not understand why we initialized to undef and had initialized to nil. Missed this when I realized why and reverted all but this place.

}

public void setBacktraceData(BacktraceData backtraceData) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/RubyKernel.java
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ public static IRubyObject raise(ThreadContext context, IRubyObject recv, IRubyOb
printExceptionSummary(runtime, raise.getException());
}

if (forceCause || argc > 0 && raise.getException().cause == UNDEF && cause != raise.getException()) {
if (forceCause || argc > 0 && raise.getException().getCause() == UNDEF && cause != raise.getException()) {
raise.getException().setCause(cause);
}

Expand Down

2 comments on commit 64d4fd8

@headius
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit broke cause logic in raise because .cause == UNDEF may be true, but getCause() returns null in that case. Fixing on master.

@enebo
Copy link
Member Author

@enebo enebo commented on 64d4fd8 Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@headius that may be true but I made another commit after this which should have fixed it.

Please sign in to comment.