From d89dec00eb4ef69fdd79e54fcc581ef5100d3ea7 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Sun, 10 May 2026 18:14:52 -0400 Subject: [PATCH 1/2] CoroutineUtils.kt added with function `createSupervisorCoroutineScope()` and code modified to use it, replacing bespoke scope creation logic --- .../DataConnectCredentialsTokenManager.kt | 16 ++--- .../core/FirebaseDataConnectImpl.kt | 18 +---- .../dataconnect/querymgr/LiveQuery.kt | 12 ++-- .../sqlite/DataConnectCacheDatabase.kt | 18 +---- .../dataconnect/util/CoroutineUtils.kt | 69 +++++++++++++++++++ .../core/DataConnectAuthUnitTest.kt | 3 +- 6 files changed, 85 insertions(+), 51 deletions(-) create mode 100644 firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt index 90330820bfc..ab6056e9864 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt @@ -24,6 +24,7 @@ import com.google.firebase.dataconnect.core.DataConnectCredentialsTokenManager.G import com.google.firebase.dataconnect.core.Globals.toScrubbedAccessToken import com.google.firebase.dataconnect.core.LoggerGlobals.debug import com.google.firebase.dataconnect.core.LoggerGlobals.warn +import com.google.firebase.dataconnect.util.CoroutineUtils.createSupervisorCoroutineScope import com.google.firebase.dataconnect.util.SequencedReference import com.google.firebase.dataconnect.util.SequencedReference.Companion.nextSequenceNumber import com.google.firebase.inject.Deferred.DeferredHandler @@ -35,13 +36,11 @@ import kotlin.coroutines.coroutineContext import kotlin.random.Random import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Deferred import kotlinx.coroutines.Job -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.async import kotlinx.coroutines.cancel import kotlinx.coroutines.ensureActive @@ -65,15 +64,10 @@ internal sealed class DataConnectCredentialsTokenManager - logger.warn(throwable) { - "uncaught exception from a coroutine named ${context[CoroutineName]}: $throwable" - } - } + createSupervisorCoroutineScope( + context = parentCoroutineScope.coroutineContext, + logger = logger, + parent = parentCoroutineScope.coroutineContext[Job] ) private sealed interface State { diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt index 2de1d705517..459bdd97e8c 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt @@ -35,6 +35,7 @@ import com.google.firebase.dataconnect.querymgr.LiveQuery import com.google.firebase.dataconnect.querymgr.QueryManager import com.google.firebase.dataconnect.querymgr.RegisteredDataDeserializer import com.google.firebase.dataconnect.util.AlphanumericStringUtil.toAlphaNumericString +import com.google.firebase.dataconnect.util.CoroutineUtils.createSupervisorCoroutineScope import com.google.firebase.dataconnect.util.ProtoUtil.buildStructProto import com.google.firebase.dataconnect.util.ProtoUtil.calculateSha512 import com.google.firebase.util.nextAlphanumericString @@ -43,14 +44,11 @@ import java.util.concurrent.Executor import kotlin.random.Random import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineExceptionHandler -import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Deferred import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async import kotlinx.coroutines.cancel @@ -109,19 +107,7 @@ internal class FirebaseDataConnectImpl( override val blockingDispatcher = blockingExecutor.asCoroutineDispatcher() override val nonBlockingDispatcher = nonBlockingExecutor.asCoroutineDispatcher() - override val coroutineScope = - CoroutineScope( - SupervisorJob() + - nonBlockingDispatcher + - CoroutineName(instanceId) + - CoroutineExceptionHandler { context, throwable -> - logger.warn(throwable) { - val coroutineName = context[CoroutineName]?.name - "WARNING: uncaught exception from coroutine named \"$coroutineName\" " + - "(error code jszxcbe37k)" - } - } - ) + override val coroutineScope = createSupervisorCoroutineScope(nonBlockingDispatcher, logger) override val connectorResourceName = "projects/$projectId/" + diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/querymgr/LiveQuery.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/querymgr/LiveQuery.kt index 55ac3f148a9..0a323765bf7 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/querymgr/LiveQuery.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/querymgr/LiveQuery.kt @@ -23,6 +23,7 @@ import com.google.firebase.dataconnect.core.DataConnectGrpcClient.OperationResul import com.google.firebase.dataconnect.core.Logger import com.google.firebase.dataconnect.core.LoggerGlobals.Logger import com.google.firebase.dataconnect.core.LoggerGlobals.debug +import com.google.firebase.dataconnect.util.CoroutineUtils.createSupervisorCoroutineScope import com.google.firebase.dataconnect.util.ImmutableByteArray import com.google.firebase.dataconnect.util.NullableReference import com.google.firebase.dataconnect.util.SequencedReference @@ -33,10 +34,8 @@ import com.google.protobuf.Struct import java.util.concurrent.CopyOnWriteArrayList import kotlin.random.Random import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.async import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableStateFlow @@ -68,10 +67,11 @@ internal class LiveQuery( } private val coroutineScope = - CoroutineScope( - SupervisorJob(parentCoroutineScope.coroutineContext[Job]) + - nonBlockingCoroutineDispatcher + - CoroutineName("LiveQuery[${logger.nameWithId}]") + createSupervisorCoroutineScope( + nonBlockingCoroutineDispatcher, + logger, + parent = parentCoroutineScope.coroutineContext[Job], + coroutineName = "LiveQuery[${logger.nameWithId}]", ) // The `dataDeserializers` list may be safely read concurrently from multiple threads, as it uses diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/sqlite/DataConnectCacheDatabase.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/sqlite/DataConnectCacheDatabase.kt index 56b6703295c..daa094c922e 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/sqlite/DataConnectCacheDatabase.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/sqlite/DataConnectCacheDatabase.kt @@ -25,6 +25,7 @@ import com.google.firebase.dataconnect.sqlite.SQLiteDatabaseExts.execSQL import com.google.firebase.dataconnect.sqlite.SQLiteDatabaseExts.getLastInsertRowId import com.google.firebase.dataconnect.sqlite.SQLiteDatabaseExts.rawQuery import com.google.firebase.dataconnect.util.BigIntegerUtil.LONG_MAX_VALUE_BIG_INTEGER +import com.google.firebase.dataconnect.util.CoroutineUtils.createSupervisorCoroutineScope import com.google.firebase.dataconnect.util.ImmutableByteArray import com.google.protobuf.Duration as DurationProto import com.google.protobuf.Struct @@ -38,13 +39,10 @@ import kotlin.reflect.KClass import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.nanoseconds -import kotlinx.coroutines.CoroutineExceptionHandler -import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.ExecutorCoroutineDispatcher import kotlinx.coroutines.Job -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async import kotlinx.coroutines.cancel @@ -97,19 +95,7 @@ internal class DataConnectCacheDatabase( Executors.newSingleThreadExecutor { runnable -> Thread(runnable, logger.nameWithId) } .asCoroutineDispatcher() - val coroutineJob = SupervisorJob() - - val coroutineScope = - CoroutineScope( - coroutineJob + - CoroutineName(logger.nameWithId) + - coroutineDispatcher + - CoroutineExceptionHandler { context, throwable -> - logger.warn(throwable) { - "uncaught exception from a coroutine named ${context[CoroutineName]}: $throwable" - } - } - ) + val coroutineScope = createSupervisorCoroutineScope(coroutineDispatcher, logger) val initializeJob = coroutineScope.async { diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt new file mode 100644 index 00000000000..0567379977c --- /dev/null +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt @@ -0,0 +1,69 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.dataconnect.util + +import com.google.firebase.dataconnect.core.Logger +import com.google.firebase.dataconnect.core.LoggerGlobals.warn +import kotlin.coroutines.CoroutineContext +import kotlin.coroutines.EmptyCoroutineContext +import kotlinx.coroutines.CoroutineExceptionHandler +import kotlinx.coroutines.CoroutineName +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob + +internal object CoroutineUtils { + + /** + * Creates and returns a new [CoroutineScope] with a [SupervisorJob], [CoroutineName], and + * [CoroutineExceptionHandler]. + * + * The [CoroutineContext] of the returned [CoroutineScope] will be the given [context], with some + * elements unconditionally replaced: + * + * * The [Job] element will be a newly-created [SupervisorJob] with the given [parent]; notably, + * if the given [parent] is null then the parent of the [SupervisorJob] will _also_ be null. + * * The [CoroutineName] element will be a newly-created instance whose value is the string given + * for the [coroutineName] argument, or the value returned from [Logger.nameWithId] if the given + * [coroutineName] is null. + * * The [CoroutineExceptionHandler] will be a newly-created instance that simply logs a warning + * message to the given [Logger] and then drops the exception. This prevents any crashing + * coroutines in the returned scope from propagating outside the scope. + */ + fun createSupervisorCoroutineScope( + context: CoroutineContext = EmptyCoroutineContext, + logger: Logger, + parent: Job? = null, + coroutineName: String? = null + ): CoroutineScope { + var newContext = context + SupervisorJob(parent) + + newContext += CoroutineExceptionHandler { exceptionContext, throwable -> + logger.warn(throwable) { + "uncaught exception from a coroutine named ${exceptionContext[CoroutineName]?.name}" + } + } + + if (coroutineName !== null) { + newContext += CoroutineName(coroutineName) + } else if (newContext[CoroutineName] === null) { + newContext += CoroutineName(logger.nameWithId) + } + + return CoroutineScope(newContext) + } +} diff --git a/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt b/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt index 75017a20579..8f9055cf981 100644 --- a/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt +++ b/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt @@ -579,8 +579,7 @@ class DataConnectAuthUnitTest { dataConnectAuth.close() withClue("result=$result") { result.shouldBeNull() } - mockLogger.shouldHaveLoggedExactlyOneMessageContaining("$testException") - mockLogger.shouldHaveLoggedExactlyOneMessageContaining("k6rwgqg9gh") + mockLogger.shouldHaveLoggedExactlyOneMessageContaining("k6rwgqg9gh", testException) mockLogger.shouldHaveLoggedExactlyOneMessageContaining( "${dataConnectAuth.instanceId} whenAvailable" ) From c20282be9a3000d8c678355e993e1d9da0064d58 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Sun, 10 May 2026 19:07:18 -0400 Subject: [PATCH 2/2] CoroutineUtils.kt: refactor the implementation of createSupervisorCoroutineScope(), and fix the CoroutineName bug. The "CoroutineName Bug" was identified by gemini code assist: The implementation of CoroutineName assignment does not match the docstring and introduces a regression in behavior. The docstring states that the CoroutineName will be unconditionally replaced, but the current implementation only sets it if it is missing from the context. This causes child scopes (like the one in DataConnectCredentialsTokenManager) to inherit the parent's name instead of using their own instanceId, which can make debugging and log analysis more difficult. --- .../dataconnect/util/CoroutineUtils.kt | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt index 0567379977c..90adf9c03f3 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/util/CoroutineUtils.kt @@ -49,21 +49,15 @@ internal object CoroutineUtils { logger: Logger, parent: Job? = null, coroutineName: String? = null - ): CoroutineScope { - var newContext = context + SupervisorJob(parent) - - newContext += CoroutineExceptionHandler { exceptionContext, throwable -> - logger.warn(throwable) { - "uncaught exception from a coroutine named ${exceptionContext[CoroutineName]?.name}" - } - } - - if (coroutineName !== null) { - newContext += CoroutineName(coroutineName) - } else if (newContext[CoroutineName] === null) { - newContext += CoroutineName(logger.nameWithId) - } - - return CoroutineScope(newContext) - } + ): CoroutineScope = + CoroutineScope( + context + + SupervisorJob(parent) + + CoroutineName(coroutineName ?: logger.nameWithId) + + CoroutineExceptionHandler { exceptionContext, throwable -> + logger.warn(throwable) { + "uncaught exception from a coroutine named ${exceptionContext[CoroutineName]?.name}" + } + } + ) }