Skip to content

Commit

Permalink
8312436: CompletableFuture never completes when 'Throwable.toString()…
Browse files Browse the repository at this point in the history
…' method throws Exception

Reviewed-by: alanb
  • Loading branch information
Viktor Klang committed Jun 5, 2024
1 parent 9a8096f commit 326dbb1
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,57 @@ final boolean completeValue(T t) {
return RESULT.compareAndSet(this, null, (t == null) ? NIL : t);
}

static CompletionException wrapInCompletionException(Throwable t) {
if (t == null)
return new CompletionException();

String message;
Throwable suppressed;
try {
message = t.toString();
suppressed = null;
} catch (Throwable unknown) {
message = "";
suppressed = unknown;
}

final CompletionException wrapping = new CompletionException(message, t);

if (suppressed != null)
wrapping.addSuppressed(suppressed);

return wrapping;
}

static ExecutionException wrapInExecutionException(Throwable t) {
if (t == null)
return new ExecutionException();

String message;
Throwable suppressed;
try {
message = t.toString();
suppressed = null;
} catch (Throwable unknown) {
message = "";
suppressed = unknown;
}

final ExecutionException wrapping = new ExecutionException(message, t);

if (suppressed != null)
wrapping.addSuppressed(suppressed);

return wrapping;
}

/**
* Returns the encoding of the given (non-null) exception as a
* wrapped CompletionException unless it is one already.
*/
static AltResult encodeThrowable(Throwable x) {
return new AltResult((x instanceof CompletionException) ? x :
new CompletionException(x));
wrapInCompletionException(x));
}

/** Completes with an exceptional result, unless already completed. */
Expand All @@ -329,7 +373,7 @@ final boolean completeThrowable(Throwable x) {
*/
static Object encodeThrowable(Throwable x, Object r) {
if (!(x instanceof CompletionException))
x = new CompletionException(x);
x = wrapInCompletionException(x);
else if (r instanceof AltResult && x == ((AltResult)r).ex)
return r;
return new AltResult(x);
Expand Down Expand Up @@ -365,7 +409,7 @@ static Object encodeRelay(Object r) {
if (r instanceof AltResult
&& (x = ((AltResult)r).ex) != null
&& !(x instanceof CompletionException))
r = new AltResult(new CompletionException(x));
r = new AltResult(wrapInCompletionException(x));
return r;
}

Expand Down Expand Up @@ -393,7 +437,7 @@ private static Object reportGet(Object r, String details)
if ((x instanceof CompletionException) &&
(cause = x.getCause()) != null)
x = cause;
throw new ExecutionException(x);
throw wrapInExecutionException(x);
}
return r;
}
Expand All @@ -410,7 +454,7 @@ private static Object reportJoin(Object r, String details) {
throw new CancellationException(details, (CancellationException)x);
if (x instanceof CompletionException)
throw (CompletionException)x;
throw new CompletionException(x);
throw wrapInCompletionException(x);
}
return r;
}
Expand Down Expand Up @@ -2605,8 +2649,8 @@ public int getNumberOfDependents() {
/**
* Returns a string identifying this CompletableFuture, as well as
* its completion state. The state, in brackets, contains the
* String {@code "Completed Normally"} or the String {@code
* "Completed Exceptionally"}, or the String {@code "Not
* String {@code "Completed normally"} or the String {@code
* "Completed exceptionally"}, or the String {@code "Not
* completed"} followed by the number of CompletableFutures
* dependent upon its completion, if any.
*
Expand All @@ -2623,7 +2667,7 @@ public String toString() {
? "[Not completed]"
: "[Not completed, " + count + " dependents]")
: (((r instanceof AltResult) && ((AltResult)r).ex != null)
? "[Completed exceptionally: " + ((AltResult)r).ex + "]"
? "[Completed exceptionally: " + ((AltResult)r).ex.getClass().getName() + "]"
: "[Completed normally]"));
}

Expand Down
12 changes: 9 additions & 3 deletions test/jdk/java/util/concurrent/tck/CompletableFutureTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,14 @@ public static Test suite() {
return new TestSuite(CompletableFutureTest.class);
}

static class CFException extends RuntimeException {}
static class CFException extends RuntimeException {
// This makes sure that CompletableFuture still behaves appropriately
// even if thrown exceptions end up throwing exceptions from their String
// representations.
@Override public String getMessage() {
throw new IllegalStateException("malformed");
}
}

void checkIncomplete(CompletableFuture<?> f) {
assertFalse(f.isDone());
Expand Down Expand Up @@ -272,8 +279,8 @@ public void testComplete() {
*/
public void testCompleteExceptionally() {
CompletableFuture<Item> f = new CompletableFuture<>();
CFException ex = new CFException();
checkIncomplete(f);
CFException ex = new CFException();
f.completeExceptionally(ex);
checkCompletedExceptionally(f, ex);
}
Expand Down Expand Up @@ -5142,5 +5149,4 @@ public void testDefaultExceptionallyComposeAsyncExecutor_actionFailed() {
checkCompletedWithWrappedException(g.toCompletableFuture(), r.ex);
r.assertInvoked();
}}

}

1 comment on commit 326dbb1

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.