From 65d946b9cc6dc2d8d56d2bdc82392621c23a5d75 Mon Sep 17 00:00:00 2001 From: Daniel Alome Date: Sun, 11 Oct 2026 21:17:03 +0100 Subject: [PATCH] ADFA-5422: Load each color scheme once, under a lock Opening a project restores its editor tabs together, and each editor reads the color scheme on its own IO coroutine. IDEColorSchemeProvider caches the scheme in an unsynchronized lazy getter, so two editors can both call IDEColorScheme.load() on the same instance at once. Both parse into the same MutableIntIntMap and ++colorId counter, which are not thread-safe. parseColorValue() caught any failure from putColor() and returned the raw ARGB color as the color id, so a corrupted write became an out-of-range id that StyleDef.makeStyle() passed to TextStyle.makeStyle(), which throws "color id must be positive and bit count is less than 19" (GlitchTip CODEONTHEGO-K, 42 fatal events, still on C-r-1006). Other interleavings leave the map's probe loop spinning forever. load() is now synchronized and returns once the scheme is loaded, so the second editor waits for the first and reuses its result. Nothing reloads an instance on purpose: load() is only reached through loadColorScheme() while the provider's cached scheme is still null. putColor() failures now propagate to IDEColorSchemeProvider.loadColorScheme(), which logs them and falls back to the default scheme, instead of surfacing later as a crash. --- .../editor/schemes/IDEColorScheme.kt | 4 ++ .../editor/schemes/internal/parser/common.kt | 2 +- .../editor/schemes/IDEColorSchemeLoadTest.kt | 56 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 editor/src/test/java/com/itsaky/androidide/editor/schemes/IDEColorSchemeLoadTest.kt diff --git a/editor/src/main/java/com/itsaky/androidide/editor/schemes/IDEColorScheme.kt b/editor/src/main/java/com/itsaky/androidide/editor/schemes/IDEColorScheme.kt index b242172975..7aa9f0f0d5 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/schemes/IDEColorScheme.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/schemes/IDEColorScheme.kt @@ -50,9 +50,13 @@ class IDEColorScheme( internal set private var colorId = endColorId + private var loaded = false + @Synchronized internal fun load() { + if (loaded) return SchemeParser { name -> File(this.file.parentFile, name) }.load(this) + loaded = true } fun getLanguageScheme(type: String): LanguageScheme? = this.languages[type] ?: this.languages[GENERIC_LANGUAGE_TYPE] diff --git a/editor/src/main/java/com/itsaky/androidide/editor/schemes/internal/parser/common.kt b/editor/src/main/java/com/itsaky/androidide/editor/schemes/internal/parser/common.kt index eda0832617..3018b2742c 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/schemes/internal/parser/common.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/schemes/internal/parser/common.kt @@ -60,7 +60,7 @@ if (value[0] == '#') { } catch (err: Throwable) { throw ParseException("Invalid hex color code: '$value'", err) } - return if (colorId) try {putColor(color)}catch (err: Throwable) {color} else color + return if (colorId) putColor(color) else color } throw ParseException("Unsupported color value '$value'") diff --git a/editor/src/test/java/com/itsaky/androidide/editor/schemes/IDEColorSchemeLoadTest.kt b/editor/src/test/java/com/itsaky/androidide/editor/schemes/IDEColorSchemeLoadTest.kt new file mode 100644 index 0000000000..427abd39f6 --- /dev/null +++ b/editor/src/test/java/com/itsaky/androidide/editor/schemes/IDEColorSchemeLoadTest.kt @@ -0,0 +1,56 @@ +package com.itsaky.androidide.editor.schemes + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File +import java.util.Collections +import java.util.concurrent.CountDownLatch +import kotlin.concurrent.thread + +@RunWith(RobolectricTestRunner::class) +class IDEColorSchemeLoadTest { + private fun newScheme() = IDEColorScheme(File("./src/test/resources/syntax.json"), "test").apply { name = "Test" } + + @Test + fun `loading a scheme again does not parse it again`() { + val scheme = newScheme() + scheme.load() + val colorsAfterFirstLoad = scheme.colorIds.size + + scheme.load() + + assertThat(scheme.colorIds.size).isEqualTo(colorsAfterFirstLoad) + } + + @Test + fun `concurrent loads of one scheme leave every style with a valid colour`() { + repeat(ROUNDS) { + val scheme = newScheme() + val failures = Collections.synchronizedList(mutableListOf()) + val start = CountDownLatch(1) + val loaders = + List(THREADS) { + thread(isDaemon = true) { + start.await() + runCatching { scheme.load() }.onFailure(failures::add) + } + } + start.countDown() + loaders.forEach { it.join(LOAD_TIMEOUT_MS) } + + assertThat(loaders.filter { it.isAlive }).isEmpty() + assertThat(failures).isEmpty() + scheme.languages.values.distinct().forEach { language -> + language.styles.values.forEach { style -> style.makeStyle() } + } + } + } + + private companion object { + const val ROUNDS = 200 + const val THREADS = 8 + const val LOAD_TIMEOUT_MS = 10_000L + } +}