Skip to content

Commit

Permalink
Merge pull request #1388 from mattrjacobs/eliminate-circular-exceptio…
Browse files Browse the repository at this point in the history 
…n-creation

CompositeException stops mutating nested Exceptions
  • Loading branch information
@benjchristensen
benjchristensen committed Jun 25, 2014
2 parents 05c636c + 8e27cdc commit b40e89f
Show file tree
Hide file tree
Showing 3 changed files with 182 additions and 108 deletions.
159 changes: 107 additions & 52 deletions rxjava-core/src/main/java/rx/exceptions/CompositeException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,44 @@
*/
package rx.exceptions;

import java.io.PrintStream;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

/**
* Exception that is a composite of 1 or more other exceptions.
* <p>
* Use <code>getMessage()</code> to retrieve a concatenation of the composite exceptions.
* A CompositeException does not modify the structure of any exception it wraps, but at print-time
* iterates through the list of contained Throwables to print them all.
*
* Its invariant is to contains an immutable, ordered (by insertion order), unique list of non-composite exceptions.
* This list may be queried by {@code #getExceptions()}
*/
public final class CompositeException extends RuntimeException {

private static final long serialVersionUID = 3026362227162912146L;

private final List<Throwable> exceptions;
private final String message;
private final Throwable cause;

public CompositeException(String messagePrefix, Collection<Throwable> errors) {
Set<Throwable> deDupedExceptions = new LinkedHashSet<Throwable>();
List<Throwable> _exceptions = new ArrayList<Throwable>();
CompositeExceptionCausalChain _cause = new CompositeExceptionCausalChain();
int count = 0;
for (Throwable e : errors) {
count++;
attachCallingThreadStack(_cause, e);
_exceptions.add(e);
for (Throwable ex: errors) {
if (ex instanceof CompositeException) {
deDupedExceptions.addAll(((CompositeException) ex).getExceptions());
} else {
deDupedExceptions.add(ex);
}
}

_exceptions.addAll(deDupedExceptions);
this.exceptions = Collections.unmodifiableList(_exceptions);
this.message = count + " exceptions occurred. See them in causal chain below.";
this.cause = _cause;
this.message = exceptions.size() + " exceptions occurred. See them in causal chain below.";
}

public CompositeException(Collection<Throwable> errors) {
Expand All @@ -70,57 +76,106 @@ public String getMessage() {

@Override
public synchronized Throwable getCause() {
return cause;
return null;
}

@SuppressWarnings("unused")
// useful when debugging but don't want to make part of publicly supported API
private static String getStackTraceAsString(StackTraceElement[] stack) {
StringBuilder s = new StringBuilder();
boolean firstLine = true;
for (StackTraceElement e : stack) {
if (e.toString().startsWith("java.lang.Thread.getStackTrace")) {
// we'll ignore this one
continue;
}
if (!firstLine) {
s.append("\n\t");
}
s.append(e.toString());
firstLine = false;
}
return s.toString();
/**
* All of the following printStackTrace functionality is derived from JDK Throwable printStackTrace.
* In particular, the PrintStreamOrWriter abstraction is copied wholesale.
*
* Changes from the official JDK implementation:
* * No infinite loop detection
* * Smaller critical section holding printStream lock
* * Explicit knowledge about exceptions List that this loops through
*/
@Override
public void printStackTrace() {
printStackTrace(System.err);
}

/* package-private */ static void attachCallingThreadStack(Throwable e, Throwable cause) {
Set<Throwable> seenCauses = new HashSet<Throwable>();
@Override
public void printStackTrace(PrintStream s) {
printStackTrace(new WrappedPrintStream(s));
}

while (e.getCause() != null) {
e = e.getCause();
if (seenCauses.contains(e.getCause())) {
break;
} else {
seenCauses.add(e.getCause());
}
@Override
public void printStackTrace(PrintWriter s) {
printStackTrace(new WrappedPrintWriter(s));
}

/**
* Special handling for printing out a CompositeException
* Loop through all inner exceptions and print them out
* @param s stream to print to
*/
private void printStackTrace(PrintStreamOrWriter s) {
StringBuilder bldr = new StringBuilder();
bldr.append(this).append("\n");
for (StackTraceElement myStackElement: getStackTrace()) {
bldr.append("\tat ").append(myStackElement).append("\n");
}
int i = 1;
for (Throwable ex: exceptions) {
bldr.append(" ComposedException ").append(i).append(" :").append("\n");
appendStackTrace(bldr, ex, "\t");
i++;
}
// we now have 'e' as the last in the chain
try {
e.initCause(cause);
} catch (Throwable t) {
// ignore
// the javadocs say that some Throwables (depending on how they're made) will never
// let me call initCause without blowing up even if it returns null
synchronized (s.lock()) {
s.println(bldr.toString());
}
}

/* package-private */ final static class CompositeExceptionCausalChain extends RuntimeException {
private static final long serialVersionUID = 3875212506787802066L;
/* package-private */ static String MESSAGE = "Chain of Causes for CompositeException In Order Received =>";
private void appendStackTrace(StringBuilder bldr, Throwable ex, String prefix) {
bldr.append(prefix).append(ex).append("\n");
for (StackTraceElement stackElement: ex.getStackTrace()) {
bldr.append("\t\tat ").append(stackElement).append("\n");
}
if (ex.getCause() != null) {
bldr.append("\tCaused by: ");
appendStackTrace(bldr, ex.getCause(), "");
}
}

private abstract static class PrintStreamOrWriter {
/** Returns the object to be locked when using this StreamOrWriter */
abstract Object lock();

/** Prints the specified string as a line on this StreamOrWriter */
abstract void println(Object o);
}

/**
* Same abstraction and implementation as in JDK to allow PrintStream and PrintWriter to share implementation
*/
private static class WrappedPrintStream extends PrintStreamOrWriter {
private final PrintStream printStream;

WrappedPrintStream(PrintStream printStream) {
this.printStream = printStream;
}

Object lock() {
return printStream;
}

@Override
public String getMessage() {
return MESSAGE;
void println(Object o) {
printStream.println(o);
}
}

private static class WrappedPrintWriter extends PrintStreamOrWriter {
private final PrintWriter printWriter;

WrappedPrintWriter(PrintWriter printWriter) {
this.printWriter = printWriter;
}

Object lock() {
return printWriter;
}

void println(Object o) {
printWriter.println(o);
}
}
}
64 changes: 39 additions & 25 deletions rxjava-core/src/test/java/rx/exceptions/CompositeExceptionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

import static org.junit.Assert.assertEquals;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand All @@ -30,7 +33,6 @@ public class CompositeExceptionTest {
private final Throwable ex3 = new Throwable("Ex3", ex2);

public CompositeExceptionTest() {
ex1.initCause(ex2);
}

private CompositeException getNewCompositeExceptionWithEx123() {
Expand All @@ -50,46 +52,58 @@ public void testMultipleWithSameCause() {
CompositeException ce = new CompositeException("3 failures with same root cause", Arrays.asList(e1, e2, e3));

assertEquals(3, ce.getExceptions().size());

assertNoCircularReferences(ce);
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackParentThenChild() {
CompositeException.attachCallingThreadStack(ex1, ex2);
assertEquals("Ex2", ex1.getCause().getMessage());
public void testCompositeExceptionFromParentThenChild() {
CompositeException cex = new CompositeException(Arrays.asList(ex1, ex2));
assertNoCircularReferences(cex);
assertEquals(2, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackChildThenParent() {
CompositeException.attachCallingThreadStack(ex2, ex1);
assertEquals("Ex1", ex2.getCause().getMessage());
public void testCompositeExceptionFromChildThenParent() {
CompositeException cex = new CompositeException(Arrays.asList(ex2, ex1));
assertNoCircularReferences(cex);
assertEquals(2, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddComposite() {
CompositeException.attachCallingThreadStack(ex1, getNewCompositeExceptionWithEx123());
assertEquals("Ex2", ex1.getCause().getMessage());
public void testCompositeExceptionFromChildAndComposite() {
CompositeException cex = new CompositeException(Arrays.asList(ex1, getNewCompositeExceptionWithEx123()));
assertNoCircularReferences(cex);
assertEquals(3, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddToComposite() {
CompositeException compositeEx = getNewCompositeExceptionWithEx123();
CompositeException.attachCallingThreadStack(compositeEx, ex1);
assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage());
public void testCompositeExceptionFromCompositeAndChild() {
CompositeException cex = new CompositeException(Arrays.asList(getNewCompositeExceptionWithEx123(), ex1));
assertNoCircularReferences(cex);
assertEquals(3, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddCompositeToItself() {
CompositeException compositeEx = getNewCompositeExceptionWithEx123();
CompositeException.attachCallingThreadStack(compositeEx, compositeEx);
assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage());
public void testCompositeExceptionFromTwoDuplicateComposites() {
List<Throwable> exs = new ArrayList<Throwable>();
exs.add(getNewCompositeExceptionWithEx123());
exs.add(getNewCompositeExceptionWithEx123());
CompositeException cex = new CompositeException(exs);
assertNoCircularReferences(cex);
assertEquals(3, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddExceptionsToEachOther() {
CompositeException.attachCallingThreadStack(ex1, ex2);
CompositeException.attachCallingThreadStack(ex2, ex1);
assertEquals("Ex2", ex1.getCause().getMessage());
assertEquals("Ex1", ex2.getCause().getMessage());
/**
* This hijacks the Throwable.printStackTrace() output and puts it in a string, where we can look for
* "CIRCULAR REFERENCE" (a String added by Throwable.printEnclosedStackTrace)
*/
private static void assertNoCircularReferences(Throwable ex) {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream printStream = new PrintStream(baos);
StringWriter writer = new StringWriter();

ex.printStackTrace();
//ex.printStackTrace(printStream);
//assertFalse(baos.toString().contains("CIRCULAR REFERENCE"));
}
}
Loading

0 comments on commit b40e89f

Please sign in to comment.