Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace "handle" with "tryHandle" for uncaught exceptions #3554

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -27,7 +27,7 @@ class ListAllCoroutineThrowableSubclassesTest {
"kotlinx.coroutines.JobCancellationException",
"kotlinx.coroutines.internal.UndeliveredElementException",
"kotlinx.coroutines.CompletionHandlerException",
"kotlinx.coroutines.DiagnosticCoroutineContextException",
"kotlinx.coroutines.internal.DiagnosticCoroutineContextException",
"kotlinx.coroutines.CoroutinesInternalError",
"kotlinx.coroutines.channels.ClosedSendChannelException",
"kotlinx.coroutines.channels.ClosedReceiveChannelException",
Expand Down
3 changes: 3 additions & 0 deletions kotlinx-coroutines-core/api/kotlinx-coroutines-core.api
Expand Up @@ -182,13 +182,16 @@ public final class kotlinx/coroutines/CoroutineDispatcher$Key : kotlin/coroutine
public abstract interface class kotlinx/coroutines/CoroutineExceptionHandler : kotlin/coroutines/CoroutineContext$Element {
public static final field Key Lkotlinx/coroutines/CoroutineExceptionHandler$Key;
public abstract fun handleException (Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)V
public abstract fun tryHandleException (Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)Z
}

public final class kotlinx/coroutines/CoroutineExceptionHandler$DefaultImpls {
public static fun fold (Lkotlinx/coroutines/CoroutineExceptionHandler;Ljava/lang/Object;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object;
public static fun get (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext$Key;)Lkotlin/coroutines/CoroutineContext$Element;
public static fun handleException (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)V
public static fun minusKey (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext$Key;)Lkotlin/coroutines/CoroutineContext;
public static fun plus (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext;)Lkotlin/coroutines/CoroutineContext;
public static fun tryHandleException (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)Z
}

public final class kotlinx/coroutines/CoroutineExceptionHandler$Key : kotlin/coroutines/CoroutineContext$Key {
Expand Down
35 changes: 26 additions & 9 deletions kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt
Expand Up @@ -4,10 +4,9 @@

package kotlinx.coroutines

import kotlinx.coroutines.internal.*
import kotlin.coroutines.*

internal expect fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable)

/**
* Helper function for coroutine builder implementations to handle uncaught and unexpected exceptions in coroutines,
* that could not be otherwise handled in a normal way through structured concurrency, saving them to a future, and
Expand All @@ -22,15 +21,15 @@ public fun handleCoroutineException(context: CoroutineContext, exception: Throwa
// Invoke an exception handler from the context if present
try {
context[CoroutineExceptionHandler]?.let {
it.handleException(context, exception)
return
if (it.tryHandleException(context, exception))
return
}
} catch (t: Throwable) {
handleCoroutineExceptionImpl(context, handlerException(exception, t))
handleUncaughtCoroutineException(context, handlerException(exception, t))
return
}
// If a handler is not present in the context or an exception was thrown, fallback to the global handler
handleCoroutineExceptionImpl(context, exception)
handleUncaughtCoroutineException(context, exception)
}

internal fun handlerException(originalException: Throwable, thrownException: Throwable): Throwable {
Expand All @@ -47,8 +46,8 @@ internal fun handlerException(originalException: Throwable, thrownException: Thr
@Suppress("FunctionName")
public inline fun CoroutineExceptionHandler(crossinline handler: (CoroutineContext, Throwable) -> Unit): CoroutineExceptionHandler =
object : AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler {
override fun handleException(context: CoroutineContext, exception: Throwable) =
handler.invoke(context, exception)
override fun tryHandleException(context: CoroutineContext, exception: Throwable): Boolean =
true.also { handler.invoke(context, exception) }
}

/**
Expand Down Expand Up @@ -105,5 +104,23 @@ public interface CoroutineExceptionHandler : CoroutineContext.Element {
* Handles uncaught [exception] in the given [context]. It is invoked
* if coroutine has an uncaught exception.
*/
public fun handleException(context: CoroutineContext, exception: Throwable)
@Deprecated(
"Use tryHandleException instead",
replaceWith = ReplaceWith("this.tryHandleException(context, exception)"),
level = DeprecationLevel.WARNING
)
public fun handleException(context: CoroutineContext, exception: Throwable) {
if (!tryHandleException(context, exception))
handleUncaughtCoroutineException(context, exception)
}

/**
* Handles uncaught [exception] in the given [context].
* Returns `true` if the processing was successful and no other processing is needed.
*/
public fun tryHandleException(context: CoroutineContext, exception: Throwable): Boolean {
@Suppress("DEPRECATION")
handleException(context, exception)
return true
}
}
@@ -0,0 +1,71 @@
/*
* Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.coroutines.internal

import kotlinx.coroutines.*
import kotlin.coroutines.*

/**
* The list of globally installed [CoroutineExceptionHandler] instances that will be notified of any exceptions that
* were not processed in any other manner.
*/
internal expect val platformExceptionHandlers: Collection<CoroutineExceptionHandler>

/**
* Ensures that the given [callback] is present in the [platformExceptionHandlers] list.
*/
internal expect fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler)

/**
* The platform-dependent global exception handler, used so that the exception is logged at least *somewhere*.
*/
internal expect fun propagateExceptionFinalResort(exception: Throwable)

/**
* Deal with exceptions that happened in coroutines and weren't programmatically dealt with.
*
* First, it notifies every [CoroutineExceptionHandler] in the [platformExceptionHandlers] list.
* If one of them throws [ExceptionSuccessfullyProcessed], it means that that handler believes that the exception was
* dealt with sufficiently well and doesn't need any further processing.
* Otherwise, the platform-dependent global exception handler is also invoked.
*/
internal fun handleUncaughtCoroutineException(context: CoroutineContext, exception: Throwable) {
// use additional extension handlers
for (handler in platformExceptionHandlers) {
try {
if (handler.tryHandleException(context, exception))
return
} catch (t: Throwable) {
propagateExceptionFinalResort(handlerException(exception, t))
}
}

try {
exception.addSuppressed(DiagnosticCoroutineContextException(context))
} catch (e: Throwable) {
// addSuppressed is never user-defined and cannot normally throw with the only exception being OOM
// we do ignore that just in case to definitely deliver the exception
}
propagateExceptionFinalResort(exception)
}

/**
* Private exception that is added to suppressed exceptions of the original exception
* when it is reported to the last-ditch current thread 'uncaughtExceptionHandler'.
*
* The purpose of this exception is to add an otherwise inaccessible diagnostic information and to
* be able to poke the context of the failing coroutine in the debugger.
*/
internal expect class DiagnosticCoroutineContextException(context: CoroutineContext) : RuntimeException

/**
* A dummy exception that signifies that the exception was successfully processed by the handler and no further
* action is required.
*
* Would be nicer if [CoroutineExceptionHandler] could return a boolean, but that would be a breaking change.
* For now, we will take solace in knowledge that such exceptions are exceedingly rare, even rarer than globally
* uncaught exceptions in general.
*/
internal object ExceptionSuccessfullyProcessed: Exception()
12 changes: 0 additions & 12 deletions kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt

This file was deleted.

@@ -0,0 +1,26 @@
/*
* Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.coroutines.internal

import kotlinx.coroutines.*
import kotlin.coroutines.*

private val platformExceptionHandlers_ = mutableSetOf<CoroutineExceptionHandler>()

internal actual val platformExceptionHandlers: Collection<CoroutineExceptionHandler>
get() = platformExceptionHandlers_

internal actual fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) {
platformExceptionHandlers_ += callback
}

internal actual fun propagateExceptionFinalResort(exception: Throwable) {
// log exception
console.error(exception)
}

internal actual class DiagnosticCoroutineContextException actual constructor(context: CoroutineContext) :
RuntimeException(context.toString())

62 changes: 0 additions & 62 deletions kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt

This file was deleted.

@@ -0,0 +1,49 @@
/*
* Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.coroutines.internal

import java.util.*
import kotlinx.coroutines.*
import kotlin.coroutines.*

/**
* A list of globally installed [CoroutineExceptionHandler] instances.
*
* Note that Android may have dummy [Thread.contextClassLoader] which is used by one-argument [ServiceLoader.load] function,
* see (https://stackoverflow.com/questions/13407006/android-class-loader-may-fail-for-processes-that-host-multiple-applications).
* So here we explicitly use two-argument `load` with a class-loader of [CoroutineExceptionHandler] class.
*
* We are explicitly using the `ServiceLoader.load(MyClass::class.java, MyClass::class.java.classLoader).iterator()`
* form of the ServiceLoader call to enable R8 optimization when compiled on Android.
*/
internal actual val platformExceptionHandlers: Collection<CoroutineExceptionHandler> = ServiceLoader.load(
CoroutineExceptionHandler::class.java,
CoroutineExceptionHandler::class.java.classLoader
).iterator().asSequence().toList()

internal actual fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) {
// we use JVM's mechanism of ServiceLoader, so this should be a no-op on JVM.
// The only thing we do is make sure that the ServiceLoader did work correctly.
check(callback in platformExceptionHandlers) { "Exception handler was not found via a ServiceLoader" }
}

internal actual fun propagateExceptionFinalResort(exception: Throwable) {
// use the thread's handler
val currentThread = Thread.currentThread()
currentThread.uncaughtExceptionHandler.uncaughtException(currentThread, exception)
}

// This implementation doesn't store a stacktrace, which is good because a stacktrace doesn't make sense for this.
internal actual class DiagnosticCoroutineContextException actual constructor(@Transient private val context: CoroutineContext) : RuntimeException() {
override fun getLocalizedMessage(): String {
return context.toString()
}

override fun fillInStackTrace(): Throwable {
// Prevent Android <= 6.0 bug, #1866
stackTrace = emptyArray()
return this
}
}
2 changes: 1 addition & 1 deletion kotlinx-coroutines-core/jvm/test/exceptions/Exceptions.kt
Expand Up @@ -39,7 +39,7 @@ class CapturingHandler : AbstractCoroutineContextElement(CoroutineExceptionHandl
{
private var unhandled: ArrayList<Throwable>? = ArrayList()

override fun handleException(context: CoroutineContext, exception: Throwable) = synchronized<Unit>(this) {
override fun tryHandleException(context: CoroutineContext, exception: Throwable) = synchronized(this) {
unhandled!!.add(exception)
}

Expand Down

This file was deleted.

@@ -0,0 +1,31 @@
/*
* Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.coroutines.internal

import kotlinx.coroutines.*
import kotlin.coroutines.*
import kotlin.native.*

private val lock = SynchronizedObject()

internal actual val platformExceptionHandlers: Collection<CoroutineExceptionHandler>
get() = synchronized(lock) { platformExceptionHandlers_ }

private val platformExceptionHandlers_ = mutableSetOf<CoroutineExceptionHandler>()

internal actual fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) {
synchronized(lock) {
platformExceptionHandlers_ += callback
}
}

@OptIn(ExperimentalStdlibApi::class)
internal actual fun propagateExceptionFinalResort(exception: Throwable) {
// log exception
processUnhandledException(exception)
}

internal actual class DiagnosticCoroutineContextException actual constructor(context: CoroutineContext) :
RuntimeException(context.toString())
10 changes: 10 additions & 0 deletions kotlinx-coroutines-test/common/src/TestScope.kt
Expand Up @@ -220,6 +220,14 @@ internal class TestScopeImpl(context: CoroutineContext) :
throw IllegalStateException("Only a single call to `runTest` can be performed during one test.")
entered = true
check(!finished)
/** the order is important: [reportException] is only guaranteed not to throw if [entered] is `true` but
* [finished] is `false`.
* However, we also want [uncaughtExceptions] to be queried after the callback is registered,
* because the exception collector will be able to report the exceptions that arrived before this test but
* after the previous one, and learning about such exceptions as soon is possible is nice. */
@Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
run { ensurePlatformExceptionHandlerLoaded(ExceptionCollector) }
ExceptionCollector.addOnExceptionCallback(lock, this::reportException)
uncaughtExceptions
}
if (exceptions.isNotEmpty()) {
Expand All @@ -234,6 +242,8 @@ internal class TestScopeImpl(context: CoroutineContext) :
fun leave(): List<Throwable> {
val exceptions = synchronized(lock) {
check(entered && !finished)
/** After [finished] becomes `true`, it is no longer valid to have [reportException] as the callback. */
ExceptionCollector.removeOnExceptionCallback(lock)
finished = true
uncaughtExceptions
}
Expand Down