From 99255a075ff013c22a02fae6d4b1db493bed13aa Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 14:10:25 -0700 Subject: [PATCH 1/8] style: spotless reformat of the tree-sitter analyzer files, no functional change ADFA-5401 touches these two, which enrolls them in the `ratchetFrom = origin/stage` ratchet and reformats each in full: this is vendored sora-editor code, 2-space indented, and becomes tabs. Two ktlint errors the ratchet surfaced in LineSpansGenerator.kt, both in the license header and neither touching the license text: the upstream sora-editor banner opened with `/**`, which parses as a dangling toplevel KDoc, and it sat directly after AndroidIDE's own header, which trips no-consecutive-comments. The two headers are now one block separated by a rule. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../sora/editor/ts/LineSpansGenerator.kt | 802 ++++++++++-------- .../rosemoe/sora/editor/ts/TsAnalyzeWorker.kt | 669 ++++++++------- 2 files changed, 770 insertions(+), 701 deletions(-) diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt index 6abcb94a3e..8517e78bff 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt @@ -13,9 +13,9 @@ * * You should have received a copy of the GNU General Public License * along with AndroidIDE. If not, see . - */ - -/******************************************************************************* + * + * ----------------------------------------------------------------------------- + * * sora-editor - the awesome code editor for Android * https://github.com/Rosemoe/sora-editor * Copyright (C) 2020-2023 Rosemoe @@ -37,7 +37,7 @@ * * Please contact Rosemoe by email 2073412493@qq.com if you need * additional information or have any questions - ******************************************************************************/ + */ package io.github.rosemoe.sora.editor.ts @@ -45,6 +45,8 @@ import android.os.Handler import android.os.Looper import android.util.Log import android.util.LruCache +import com.itsaky.androidide.plugins.extensions.DecorationSpan +import com.itsaky.androidide.syntax.decoration.EditorDecorationRegistry import com.itsaky.androidide.treesitter.TSInputEdit import com.itsaky.androidide.treesitter.TSQueryCapture import com.itsaky.androidide.treesitter.TSQueryCursor @@ -61,14 +63,12 @@ import io.github.rosemoe.sora.lang.styling.span.SpanExtAttrs import io.github.rosemoe.sora.text.CharPosition import io.github.rosemoe.sora.text.Content import io.github.rosemoe.sora.widget.schemes.EditorColorScheme -import com.itsaky.androidide.plugins.extensions.DecorationSpan -import com.itsaky.androidide.syntax.decoration.EditorDecorationRegistry -import java.util.TreeMap import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancel import kotlinx.coroutines.launch +import java.util.TreeMap import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.Executors import java.util.concurrent.atomic.AtomicBoolean @@ -81,380 +81,430 @@ import java.util.concurrent.atomic.AtomicInteger * * @author Rosemoe */ -class LineSpansGenerator(internal var tree: TSTree, internal var lineCount: Int, - private val content: Content, internal var theme: TsTheme, - private val languageSpec: TsLanguageSpec, var scopedVariables: TsScopedVariables, - private val spanFactory: TsSpanFactory, private val requestRedraw: () -> Unit) : Spans { - - companion object { - - const val CACHE_THRESHOLD = 100 - const val TAG = "LineSpansGenerator" - /** - * Delay in milliseconds to batch UI redraws, preventing frame drops - * when rapidly calculating multiple lines. - */ - const val REDRAW_DEBOUNCE_DELAY_MS = 32L - } - - /** - * Thread-safe cache for calculated line spans. - * Automatically evicts the least recently used lines. - */ - private val caches = LruCache>(CACHE_THRESHOLD) - private val calculatingLines = ConcurrentHashMap.newKeySet() - - private val tsExecutor = Executors.newSingleThreadExecutor { r -> - Thread(r, "TreeSitterWorker") - } - private val tsDispatcher = tsExecutor.asCoroutineDispatcher() - private val scope = CoroutineScope(SupervisorJob() + tsDispatcher) - - /** - * Tracks content changes so the worker can instantly abort - * outdated calculations when the user types. - */ - private val contentVersion = AtomicInteger(0) - private val mainHandler = Handler(Looper.getMainLooper()) - private var isRefreshScheduled = AtomicBoolean(false) - - fun edit(edit: TSInputEdit) { - contentVersion.incrementAndGet() - scope.launch { - tree.edit(edit) - calculatingLines.clear() - } - } - - /** - * Queues the native tree destruction in the background - * so it doesn't close while a query is running. - */ - fun destroy() { - scope.cancel() - caches.evictAll() - calculatingLines.clear() - - mainHandler.removeCallbacksAndMessages(null) - - tsExecutor.execute { runCatching { tree.close() } } - tsExecutor.shutdown() - } - - fun captureRegion(startIndex: Int, endIndex: Int): MutableList { - val list = mutableListOf() - - if (!tree.canAccess() || tree.rootNode.hasChanges()) { - list.add(emptySpan(0)) - return list - } - - val captures = mutableListOf() - - TSQueryCursor.create().use { cursor -> - cursor.setByteRange(startIndex * 2, endIndex * 2) - - cursor.safeExecQueryCursor(query = languageSpec.tsQuery, tree = tree, - recycleNodeAfterUse = true, debugLogging = false, - debugName = "LineSpansGenerator.captureRegion()") { match -> - if (languageSpec.queryPredicator.doPredicate(languageSpec.predicates, content, match)) { - captures.addAll(match.captures) - } - } - - captures.sortBy { it.node.startByte } - var lastIndex = 0 - - for (capture in captures) { - val startByte = capture.node.startByte - val endByte = capture.node.endByte - val start = (startByte / 2 - startIndex).coerceAtLeast(0) - val pattern = capture.index - // Do not add span for overlapping regions and out-of-bounds regions - if (start >= lastIndex && endByte / 2 >= startIndex && startByte / 2 < endIndex && (pattern !in languageSpec.localsScopeIndices && pattern !in languageSpec.localsDefinitionIndices && pattern !in languageSpec.localsDefinitionValueIndices && pattern !in languageSpec.localsMembersScopeIndices)) { - if (start != lastIndex) { - list.addAll(createSpans(capture, lastIndex, start - 1, theme.normalTextStyle)) - } - var style = 0L - if (capture.index in languageSpec.localsReferenceIndices) { - val def = scopedVariables.findDefinition(startByte / 2, endByte / 2, - content.substring(startByte / 2, endByte / 2)) - if (def != null && def.matchedHighlightPattern != -1) { - style = theme.resolveStyleForPattern(def.matchedHighlightPattern) - } - // This reference can not be resolved to its definition - // but it can have its own fallback color by other captures - // so continue to next capture - if (style == 0L) { - continue - } - } - if (style == 0L) { - style = theme.resolveStyleForPattern(capture.index) - } - if (style == 0L) { - style = theme.normalTextStyle - } - val end = (endByte / 2 - startIndex).coerceAtMost(endIndex) - list.addAll(createSpans(capture, start, end, style)) - lastIndex = end - } - - (capture as? TreeSitterQueryCapture?)?.recycle() - } - - if (lastIndex != endIndex) { - list.add(emptySpan(lastIndex)) - } - } - if (list.isEmpty()) { - list.add(emptySpan(0)) - } - return applyDecorations(list, startIndex, endIndex) - } - - // --------------------------------------------------------------------------- - // Plugin editor decorations - // - // After the base (tree-sitter) spans for a region are built, every registered - // EditorDecorationProvider is given the region and returns additive, foreground-only color - // spans, which are merged in here. The IDE is feature-agnostic — it knows nothing about what a - // provider decorates (brackets, indent guides, markers, ...). Providers run on this analyze - // thread and receive the full document content so they can use context outside the region. - // --------------------------------------------------------------------------- - - private fun applyDecorations(list: MutableList, startIndex: Int, - endIndex: Int): MutableList { - val providers = EditorDecorationRegistry.providers() - if (providers.isEmpty() || endIndex <= startIndex) return list - - val isDark = EditorDecorationRegistry.isDark - var decorations: ArrayList? = null - for (provider in providers) { - val spans = try { - provider.decorate(content, startIndex, endIndex, isDark) - } catch (t: Throwable) { - Log.e(TAG, "Editor decoration provider failed", t) - continue - } - if (spans.isEmpty()) continue - (decorations ?: ArrayList().also { decorations = it }).addAll(spans) - } - - val decos = decorations ?: return list - return mergeDecorations(list, startIndex, endIndex, decos) - } - - /** - * Merges additive, foreground-only decoration spans into [list]: overrides only the foreground - * color of the covered characters while preserving every base style underneath, and keeps the - * strictly-ascending, non-overlapping span ordering the renderer requires. Decoration offsets are - * absolute; they are clipped to the region and converted to line-relative columns. - */ - private fun mergeDecorations(list: MutableList, startIndex: Int, endIndex: Int, - decorations: List): MutableList { - val lineLen = endIndex - startIndex - - // Snapshot of the base styles, used to restore the original style after each decorated range. - val base = TreeMap() - for (s in list) base.putIfAbsent(s.column, s.style) - val defaultStyle = TextStyle.makeStyle(EditorColorScheme.TEXT_NORMAL) - fun baseStyleAt(col: Int): Long = base.floorEntry(col)?.value ?: defaultStyle - - val result = TreeMap() - for (s in list) result.putIfAbsent(s.column, s) - - for (d in decorations) { - val s = (d.start - startIndex).coerceAtLeast(0) - val e = (d.end - startIndex).coerceAtMost(lineLen) - if (e <= s) continue - - // Override the foreground at the range start and at every span boundary inside it, so the - // color survives base-style changes within the range. Base style (bold/etc.) is preserved. - var coveredStart = false - for (col in result.subMap(s, true, e, false).keys.toList()) { - result[col] = coloredSpan(col, result[col]!!.style, d.argb) - if (col == s) coveredStart = true - } - if (!coveredStart) { - result[s] = coloredSpan(s, baseStyleAt(s), d.argb) - } - // Resume the underlying style right after the range, unless a span already begins there. - if (e < lineLen && !result.containsKey(e)) { - result[e] = SpanFactory.obtain(e, baseStyleAt(e)) - } - } - - return ArrayList(result.values) - } - - private fun coloredSpan(column: Int, style: Long, argb: Int): Span { - val span = SpanFactory.obtain(column, style) - span.setSpanExt(SpanExtAttrs.EXT_COLOR_RESOLVER, SpanConstColorResolver(argb, 0)) - return span - } - - private fun createSpans(capture: TSQueryCapture, startColumn: Int, endColumn: Int, - style: Long): List { - val spans = spanFactory.createSpans(capture, startColumn, style) - if (spans.size > 1) { - var prevCol = spans[0].column - if (prevCol > endColumn) { - throw IndexOutOfBoundsException( - "Span's column is out of bounds! column=$prevCol, endColumn=$endColumn") - } - for (i in 1..spans.lastIndex) { - val col = spans[i].column - if (col <= prevCol) { - throw IllegalStateException("Spans must not overlap! prevCol=$prevCol, col=$col") - } - if (col > endColumn) { - throw IndexOutOfBoundsException( - "Span's column is out of bounds! column=$col, endColumn=$endColumn") - } - prevCol = col - } - } - return spans - } - - private fun emptySpan(column: Int): Span { - return SpanFactory.obtain(column, TextStyle.makeStyle(EditorColorScheme.TEXT_NORMAL)) - } - - override fun adjustOnInsert(start: CharPosition, end: CharPosition) { - val lineDiff = end.line - start.line - - if (lineDiff == 0) { - val colDiff = end.column - start.column - shiftSpansOnLine(start.line, start.column, colDiff) - return - } - - rebuildCache { line, spans, cache -> - when { - line < start.line -> cache.put(line, spans) - line == start.line -> { - cache.put(line, spans) - cache.put(line + lineDiff, spans) - } - else -> cache.put(line + lineDiff, spans) - } - } - } - - override fun adjustOnDelete(start: CharPosition, end: CharPosition) { - val lineDiff = end.line - start.line - - if (lineDiff == 0) { - val colDiff = start.column - end.column - shiftSpansOnLine(start.line, end.column, colDiff) - return - } - - rebuildCache { line, spans, cache -> - when { - line < start.line -> cache.put(line, spans) - line == start.line -> cache.put(line, spans) - line > end.line -> cache.put(line - lineDiff, spans) - } - } - } - - /** - * Shifts span columns horizontally to prevent visual flickering during inline edits. - * - * @param line Line index of the modification. - * @param startColumn Column index where the shift begins. - * @param colDiff Number of columns to shift. - */ - private fun shiftSpansOnLine(line: Int, startColumn: Int, colDiff: Int) { - caches.get(line)?.forEach { span -> - if (span.column >= startColumn) { - span.column += colDiff - } - } - } - - /** - * Rebuilds the line cache for vertical text shifts (line additions or deletions). - * - * @param action Logic to determine how each cached line is re-inserted. - */ - private inline fun rebuildCache(action: (line: Int, spans: MutableList, cache: LruCache>) -> Unit) { - val snapshot = caches.snapshot() - caches.evictAll() - - for ((line, spans) in snapshot) { - action(line, spans, caches) - } - } - - override fun read() = object : Spans.Reader { - - private var spans = mutableListOf() - - override fun moveToLine(line: Int) { - spans = getSpansForLine(line) - } - - override fun getSpanCount() = spans.size - - override fun getSpanAt(index: Int) = spans[index] - override fun getSpansOnLine(line: Int): MutableList = getSpansForLine(line) - - private fun getSpansForLine(line: Int): MutableList { - if (line !in 0.. Unit, +) : Spans { + companion object { + const val CACHE_THRESHOLD = 100 + const val TAG = "LineSpansGenerator" + + /** + * Delay in milliseconds to batch UI redraws, preventing frame drops + * when rapidly calculating multiple lines. + */ + const val REDRAW_DEBOUNCE_DELAY_MS = 32L + } + +/** +* Thread-safe cache for calculated line spans. +* Automatically evicts the least recently used lines. +*/ + private val caches = LruCache>(CACHE_THRESHOLD) + private val calculatingLines = ConcurrentHashMap.newKeySet() + + private val tsExecutor = + Executors.newSingleThreadExecutor { r -> + Thread(r, "TreeSitterWorker") + } + private val tsDispatcher = tsExecutor.asCoroutineDispatcher() + private val scope = CoroutineScope(SupervisorJob() + tsDispatcher) + +/** +* Tracks content changes so the worker can instantly abort +* outdated calculations when the user types. +*/ + private val contentVersion = AtomicInteger(0) + private val mainHandler = Handler(Looper.getMainLooper()) + private var isRefreshScheduled = AtomicBoolean(false) + + fun edit(edit: TSInputEdit) { + contentVersion.incrementAndGet() + scope.launch { + tree.edit(edit) + calculatingLines.clear() + } + } + +/** +* Queues the native tree destruction in the background +* so it doesn't close while a query is running. +*/ + fun destroy() { + scope.cancel() + caches.evictAll() + calculatingLines.clear() + + mainHandler.removeCallbacksAndMessages(null) + + tsExecutor.execute { runCatching { tree.close() } } + tsExecutor.shutdown() + } + + fun captureRegion( + startIndex: Int, + endIndex: Int, + ): MutableList { + val list = mutableListOf() + + if (!tree.canAccess() || tree.rootNode.hasChanges()) { + list.add(emptySpan(0)) + return list + } + + val captures = mutableListOf() + + TSQueryCursor.create().use { cursor -> + cursor.setByteRange(startIndex * 2, endIndex * 2) + + cursor.safeExecQueryCursor( + query = languageSpec.tsQuery, + tree = tree, + recycleNodeAfterUse = true, + debugLogging = false, + debugName = "LineSpansGenerator.captureRegion()", + ) { match -> + if (languageSpec.queryPredicator.doPredicate(languageSpec.predicates, content, match)) { + captures.addAll(match.captures) + } + } + + captures.sortBy { it.node.startByte } + var lastIndex = 0 + + for (capture in captures) { + val startByte = capture.node.startByte + val endByte = capture.node.endByte + val start = (startByte / 2 - startIndex).coerceAtLeast(0) + val pattern = capture.index + // Do not add span for overlapping regions and out-of-bounds regions + if (start >= lastIndex && endByte / 2 >= startIndex && startByte / 2 < endIndex && + ( + pattern !in languageSpec.localsScopeIndices && pattern !in languageSpec.localsDefinitionIndices && + pattern !in languageSpec.localsDefinitionValueIndices && + pattern !in languageSpec.localsMembersScopeIndices + ) + ) { + if (start != lastIndex) { + list.addAll(createSpans(capture, lastIndex, start - 1, theme.normalTextStyle)) + } + var style = 0L + if (capture.index in languageSpec.localsReferenceIndices) { + val def = + scopedVariables.findDefinition( + startByte / 2, + endByte / 2, + content.substring(startByte / 2, endByte / 2), + ) + if (def != null && def.matchedHighlightPattern != -1) { + style = theme.resolveStyleForPattern(def.matchedHighlightPattern) + } + // This reference can not be resolved to its definition + // but it can have its own fallback color by other captures + // so continue to next capture + if (style == 0L) { + continue + } + } + if (style == 0L) { + style = theme.resolveStyleForPattern(capture.index) + } + if (style == 0L) { + style = theme.normalTextStyle + } + val end = (endByte / 2 - startIndex).coerceAtMost(endIndex) + list.addAll(createSpans(capture, start, end, style)) + lastIndex = end + } + + (capture as? TreeSitterQueryCapture?)?.recycle() + } + + if (lastIndex != endIndex) { + list.add(emptySpan(lastIndex)) + } + } + if (list.isEmpty()) { + list.add(emptySpan(0)) + } + return applyDecorations(list, startIndex, endIndex) + } + +// --------------------------------------------------------------------------- +// Plugin editor decorations +// +// After the base (tree-sitter) spans for a region are built, every registered +// EditorDecorationProvider is given the region and returns additive, foreground-only color +// spans, which are merged in here. The IDE is feature-agnostic — it knows nothing about what a +// provider decorates (brackets, indent guides, markers, ...). Providers run on this analyze +// thread and receive the full document content so they can use context outside the region. +// --------------------------------------------------------------------------- + + private fun applyDecorations( + list: MutableList, + startIndex: Int, + endIndex: Int, + ): MutableList { + val providers = EditorDecorationRegistry.providers() + if (providers.isEmpty() || endIndex <= startIndex) return list + + val isDark = EditorDecorationRegistry.isDark + var decorations: ArrayList? = null + for (provider in providers) { + val spans = + try { + provider.decorate(content, startIndex, endIndex, isDark) + } catch (t: Throwable) { + Log.e(TAG, "Editor decoration provider failed", t) + continue + } + if (spans.isEmpty()) continue + (decorations ?: ArrayList().also { decorations = it }).addAll(spans) + } + + val decos = decorations ?: return list + return mergeDecorations(list, startIndex, endIndex, decos) + } + +/** +* Merges additive, foreground-only decoration spans into [list]: overrides only the foreground +* color of the covered characters while preserving every base style underneath, and keeps the +* strictly-ascending, non-overlapping span ordering the renderer requires. Decoration offsets are +* absolute; they are clipped to the region and converted to line-relative columns. +*/ + private fun mergeDecorations( + list: MutableList, + startIndex: Int, + endIndex: Int, + decorations: List, + ): MutableList { + val lineLen = endIndex - startIndex + + // Snapshot of the base styles, used to restore the original style after each decorated range. + val base = TreeMap() + for (s in list) base.putIfAbsent(s.column, s.style) + val defaultStyle = TextStyle.makeStyle(EditorColorScheme.TEXT_NORMAL) + + fun baseStyleAt(col: Int): Long = base.floorEntry(col)?.value ?: defaultStyle + + val result = TreeMap() + for (s in list) result.putIfAbsent(s.column, s) + + for (d in decorations) { + val s = (d.start - startIndex).coerceAtLeast(0) + val e = (d.end - startIndex).coerceAtMost(lineLen) + if (e <= s) continue + + // Override the foreground at the range start and at every span boundary inside it, so the + // color survives base-style changes within the range. Base style (bold/etc.) is preserved. + var coveredStart = false + for (col in result.subMap(s, true, e, false).keys.toList()) { + result[col] = coloredSpan(col, result[col]!!.style, d.argb) + if (col == s) coveredStart = true + } + if (!coveredStart) { + result[s] = coloredSpan(s, baseStyleAt(s), d.argb) + } + // Resume the underlying style right after the range, unless a span already begins there. + if (e < lineLen && !result.containsKey(e)) { + result[e] = SpanFactory.obtain(e, baseStyleAt(e)) + } + } + + return ArrayList(result.values) + } + + private fun coloredSpan( + column: Int, + style: Long, + argb: Int, + ): Span { + val span = SpanFactory.obtain(column, style) + span.setSpanExt(SpanExtAttrs.EXT_COLOR_RESOLVER, SpanConstColorResolver(argb, 0)) + return span + } + + private fun createSpans( + capture: TSQueryCapture, + startColumn: Int, + endColumn: Int, + style: Long, + ): List { + val spans = spanFactory.createSpans(capture, startColumn, style) + if (spans.size > 1) { + var prevCol = spans[0].column + if (prevCol > endColumn) { + throw IndexOutOfBoundsException("Span's column is out of bounds! column=$prevCol, endColumn=$endColumn") + } + for (i in 1..spans.lastIndex) { + val col = spans[i].column + if (col <= prevCol) { + throw IllegalStateException("Spans must not overlap! prevCol=$prevCol, col=$col") + } + if (col > endColumn) { + throw IndexOutOfBoundsException("Span's column is out of bounds! column=$col, endColumn=$endColumn") + } + prevCol = col + } + } + return spans + } + + private fun emptySpan(column: Int): Span = SpanFactory.obtain(column, TextStyle.makeStyle(EditorColorScheme.TEXT_NORMAL)) + + override fun adjustOnInsert( + start: CharPosition, + end: CharPosition, + ) { + val lineDiff = end.line - start.line + + if (lineDiff == 0) { + val colDiff = end.column - start.column + shiftSpansOnLine(start.line, start.column, colDiff) + return + } + + rebuildCache { line, spans, cache -> + when { + line < start.line -> { + cache.put(line, spans) + } + + line == start.line -> { + cache.put(line, spans) + cache.put(line + lineDiff, spans) + } + + else -> { + cache.put(line + lineDiff, spans) + } + } + } + } + + override fun adjustOnDelete( + start: CharPosition, + end: CharPosition, + ) { + val lineDiff = end.line - start.line + + if (lineDiff == 0) { + val colDiff = start.column - end.column + shiftSpansOnLine(start.line, end.column, colDiff) + return + } + + rebuildCache { line, spans, cache -> + when { + line < start.line -> cache.put(line, spans) + line == start.line -> cache.put(line, spans) + line > end.line -> cache.put(line - lineDiff, spans) + } + } + } + +/** +* Shifts span columns horizontally to prevent visual flickering during inline edits. +* +* @param line Line index of the modification. +* @param startColumn Column index where the shift begins. +* @param colDiff Number of columns to shift. +*/ + private fun shiftSpansOnLine( + line: Int, + startColumn: Int, + colDiff: Int, + ) { + caches.get(line)?.forEach { span -> + if (span.column >= startColumn) { + span.column += colDiff + } + } + } + +/** +* Rebuilds the line cache for vertical text shifts (line additions or deletions). +* +* @param action Logic to determine how each cached line is re-inserted. +*/ + private inline fun rebuildCache(action: (line: Int, spans: MutableList, cache: LruCache>) -> Unit) { + val snapshot = caches.snapshot() + caches.evictAll() + + for ((line, spans) in snapshot) { + action(line, spans, caches) + } + } + + override fun read() = + object : Spans.Reader { + private var spans = mutableListOf() + + override fun moveToLine(line: Int) { + spans = getSpansForLine(line) } - return mutableListOf(emptySpan(0)) - } - } + override fun getSpanCount() = spans.size - /** - * Groups redraw requests together to avoid overloading the UI thread. - */ - private fun scheduleRefresh() { - if (!isRefreshScheduled.compareAndSet(false, true)) return + override fun getSpanAt(index: Int) = spans[index] + + override fun getSpansOnLine(line: Int): MutableList = getSpansForLine(line) + + private fun getSpansForLine(line: Int): MutableList { + if (line !in 0..>() - private var analyzerJob: Job? = null - - private var isInitialized = false - private var isDestroyed = false - - val document = TsTextDocument(languageSpec.language) - - internal val tree: TSTree? - get() = document.tree - - internal val text: UTF16String - get() = document.text - - internal fun init(init: Init) { - if (isDestroyed) { - log.warn("Received Init after TsAnalyzeWorker has been destroyed. Ignoring...") - return - } - - messageChannel.offer(init) - } - - internal fun onMod(mod: Mod) { - if (isDestroyed) { - log.warn("Received Mod after TsAnalyzeWorker has been destroyed. Ignoring...") - return - } - - messageChannel.offer(mod) - } - - fun stop() { - log.debug("Stopping TsAnalyzeWorker...") - isDestroyed = true - - document.requestCancellationAndWaitIfParsing() - - analyzerContext.close() - messageChannel.clear() - analyzerJob?.cancel(CancellationException("Requested to be stopped")) - analyzerScope.cancel(CancellationException("Requested to be stopped")) - document.close() - } - - fun start() { - check(!isDestroyed) { "TsAnalyeWorker has already been destroyed" } - - analyzerJob = analyzerScope.launch { - while (!isDestroyed && isActive) { - processNextMessage() - } - }.also { job -> - job.invokeOnCompletion { error -> - if (error != null && error !is CancellationException) { - log.error("Analyzer job failed", error) - } else { - log.info("Analyzer job completed") - } - } - } - } - - fun addBreakpoint(line: Int) = toggleBreakpoint(line = line, addOnly = true) - fun removeBreakpoint(line: Int) = toggleBreakpoint(line = line, removeOnly = true) - fun removeAllBreakpoints() { - styles.lineStyles?.forEach { style -> - style.eraseStyle(LineGutterBackground::class.java) - } - refreshLineStyles() - } - - fun toggleBreakpoint(line: Int, addOnly: Boolean = false, removeOnly: Boolean = false) { - require(!(addOnly && removeOnly)) { - "set either addOnly or removeOnly, not both" - } - - val lineStyle = styles.lineStyles?.firstOrNull { it.line == line } - val gutterBg = lineStyle?.findOne(LineGutterBackground::class.java) - var notify = true - - if (gutterBg == null && !removeOnly) { - styles.addLineStyle(LineGutterBackground(line) { scheme -> - scheme.getColor(SchemeAndroidIDE.BREAKPOINT_LINE_INDICATOR) - }) - } else if (!addOnly) { - styles.eraseLineStyle(line, LineGutterBackground::class.java) - } else { - notify = false - } - - if (notify) { - refreshLineStyles() - } - } - - fun highlightLine(line: Int) { - val lineStyle = styles.lineStyles?.firstOrNull { it.line == line } - val lineBg = lineStyle?.findOne(LineBackground::class.java) - if (lineBg == null) { - styles.addLineStyle(LineBackground(line) { scheme -> - scheme.getColor(SchemeAndroidIDE.BREAKPOINT_LINE_BG) - }) - refreshLineStyles() - } - } - - fun unhighlightLines() { - styles.lineStyles?.forEach { style -> - style.eraseStyle(LineBackground::class.java) - } - refreshLineStyles() - } - - private fun refreshLineStyles() { - // We can call styles.finishBuilding() instead of sorting lineStyles manually - // but finishBuilding() performs some unnecessary tasks like iterating over and sorting - // blockLines as well, which we don't need to do here - // As a result, we manually sort the line styles to avoid that unnecessary processing - styles.lineStyles?.sort() - stylesReceiver?.setStyles(analyzer, styles) - } - - private fun processNextMessage() { - val message = messageChannel.take() - if (isDestroyed) { - return - } - - try { - when (message) { - is Init -> doInit(message) - is Mod -> doMod(message) - } - } catch (err: Throwable) { - val langName = languageSpec.language.name - val msgType = message.javaClass.simpleName - val msgTypeSuffix = if (message is Mod) { - "[start=${message.data.start}, end=${message.data.end}, type=${if (message.data.changedText == null) "delete" else "insert"}]" - } else "" - val pendingMsgs = messageChannel.size - log.error( - "AnalyzeWorker[lang={}, message={}{}], pendingMsgs={}] crashed", - langName, - msgType, - msgTypeSuffix, - pendingMsgs, - err) - } - } - - private fun doInit(init: Init) { - document.requestCancellationAndWaitIfParsing() - - check(!isInitialized) { - "'Init' must be the first message to TsAnalyzeWorker" - } - - document.doInit(init.data) - document.reparse() - updateStyles() - - isInitialized = true - } - - private fun doMod(mod: Mod) { - - check(isInitialized) { - "'Init' must be the first message to TsAnalyzeWorker" - } - - val textMod = mod.data - val edit = textMod.edit - - val oldTree = tree!! - oldTree.edit(edit) - - document.doMod(textMod) - - (edit as? TreeSitterInputEdit?)?.recycle() - - document.requestCancellationAndWaitIfParsing() - - if (isDestroyed) { - return - } - - document.reparse(oldTree) - - oldTree.close() - updateStyles() - } - - private fun updateStyles() { - if (isDestroyed || messageChannel.isNotEmpty() || tree?.canAccess() != true) { - // analyzer stopped or - // more message need to be processed - return - } - - val tree = tree!! - val scopedVariables = TsScopedVariables(tree, text, languageSpec) - val oldSpans = (styles.spans as? LineSpansGenerator?) - val oldBrackets = analyzer.currentBracketPairs - - oldSpans?.destroy() - - // Use separate tree copies for the background worker and the UI thread - // to prevent concurrent access crashes. - styles.spans = LineSpansGenerator( - tree.copy(), - reference.lineCount, - reference.reference, - theme, - languageSpec, - scopedVariables, - spanFactory, - requestRedraw = { stylesReceiver?.setStyles(analyzer, styles) } - ) - - val newBrackets = TsBracketPairs(tree.copy(), languageSpec) - analyzer.currentBracketPairs = newBrackets - - val oldBlocks = styles.blocks - updateCodeBlocks() - oldBlocks?.also { ObjectAllocator.recycleBlockLines(it) } - - stylesReceiver?.setStyles(analyzer, styles) - stylesReceiver?.updateBracketProvider(analyzer, newBrackets) - - oldBrackets?.let { Handler(Looper.getMainLooper()).post { it.close() } } - } - - private fun updateCodeBlocks() { - if (languageSpec.blocksQuery.patternCount == 0 - || !languageSpec.blocksQuery.canAccess() - || tree?.canAccess() != true - ) { - return - } - - val blocks = mutableListOf() - TSQueryCursor.create().use { cursor -> - - cursor.safeExecQueryCursor( - query = languageSpec.blocksQuery, - tree = tree, - recycleNodeAfterUse = true, - matchCondition = { !isDestroyed }, - onClosedOrEdited = { blocks.clear() }, - debugName = "TsAnalyzeManager.updateCodeBlocks()" - ) { match -> - if (!languageSpec.blocksPredicator.doPredicate( - languageSpec.predicates, - text, - match - ) - ) { - return@safeExecQueryCursor - } - - match.captures.forEach { capture -> - val block = ObjectAllocator.obtainBlockLine() - var node = capture.node - val start = node.startPoint - - block.startLine = start.row - block.startColumn = start.column / 2 - - val end = if (languageSpec.blocksQuery.getCaptureNameForId(capture.index) - .endsWith(".marked") - ) { - // Goto last terminal element - while (node.childCount > 0) { - node = node.getChild(node.childCount - 1) - } - node.startPoint - } else { - node.endPoint - } - block.endLine = end.row - block.endColumn = end.column / 2 - if (block.endLine - block.startLine > 1) { - blocks.add(block) - } - - (capture as? TreeSitterQueryCapture?)?.recycle() - } - } - } - - val distinct = blocks.asSequence().distinct().toMutableList() - styles.blocks = distinct - styles.finishBuilding() - } + companion object { + private val log = LoggerFactory.getLogger(TsAnalyzeWorker::class.java) + } + + var stylesReceiver: StyleReceiver? = null + + @OptIn(DelicateCoroutinesApi::class, ExperimentalCoroutinesApi::class) + private val analyzerContext = newSingleThreadContext("TsAnalyzeWorkerContext") + + private val analyzerScope = CoroutineScope(analyzerContext) + private val messageChannel = LinkedBlockingQueue>() + private var analyzerJob: Job? = null + + private var isInitialized = false + private var isDestroyed = false + + val document = TsTextDocument(languageSpec.language) + + internal val tree: TSTree? + get() = document.tree + + internal val text: UTF16String + get() = document.text + + internal fun init(init: Init) { + if (isDestroyed) { + log.warn("Received Init after TsAnalyzeWorker has been destroyed. Ignoring...") + return + } + + messageChannel.offer(init) + } + + internal fun onMod(mod: Mod) { + if (isDestroyed) { + log.warn("Received Mod after TsAnalyzeWorker has been destroyed. Ignoring...") + return + } + + messageChannel.offer(mod) + } + + fun stop() { + log.debug("Stopping TsAnalyzeWorker...") + isDestroyed = true + + document.requestCancellationAndWaitIfParsing() + + analyzerContext.close() + messageChannel.clear() + analyzerJob?.cancel(CancellationException("Requested to be stopped")) + analyzerScope.cancel(CancellationException("Requested to be stopped")) + document.close() + } + + fun start() { + check(!isDestroyed) { "TsAnalyeWorker has already been destroyed" } + + analyzerJob = + analyzerScope + .launch { + while (!isDestroyed && isActive) { + processNextMessage() + } + }.also { job -> + job.invokeOnCompletion { error -> + if (error != null && error !is CancellationException) { + log.error("Analyzer job failed", error) + } else { + log.info("Analyzer job completed") + } + } + } + } + + fun addBreakpoint(line: Int) = toggleBreakpoint(line = line, addOnly = true) + + fun removeBreakpoint(line: Int) = toggleBreakpoint(line = line, removeOnly = true) + + fun removeAllBreakpoints() { + styles.lineStyles?.forEach { style -> + style.eraseStyle(LineGutterBackground::class.java) + } + refreshLineStyles() + } + + fun toggleBreakpoint( + line: Int, + addOnly: Boolean = false, + removeOnly: Boolean = false, + ) { + require(!(addOnly && removeOnly)) { + "set either addOnly or removeOnly, not both" + } + + val lineStyle = styles.lineStyles?.firstOrNull { it.line == line } + val gutterBg = lineStyle?.findOne(LineGutterBackground::class.java) + var notify = true + + if (gutterBg == null && !removeOnly) { + styles.addLineStyle( + LineGutterBackground(line) { scheme -> + scheme.getColor(SchemeAndroidIDE.BREAKPOINT_LINE_INDICATOR) + }, + ) + } else if (!addOnly) { + styles.eraseLineStyle(line, LineGutterBackground::class.java) + } else { + notify = false + } + + if (notify) { + refreshLineStyles() + } + } + + fun highlightLine(line: Int) { + val lineStyle = styles.lineStyles?.firstOrNull { it.line == line } + val lineBg = lineStyle?.findOne(LineBackground::class.java) + if (lineBg == null) { + styles.addLineStyle( + LineBackground(line) { scheme -> + scheme.getColor(SchemeAndroidIDE.BREAKPOINT_LINE_BG) + }, + ) + refreshLineStyles() + } + } + + fun unhighlightLines() { + styles.lineStyles?.forEach { style -> + style.eraseStyle(LineBackground::class.java) + } + refreshLineStyles() + } + + private fun refreshLineStyles() { + // We can call styles.finishBuilding() instead of sorting lineStyles manually + // but finishBuilding() performs some unnecessary tasks like iterating over and sorting + // blockLines as well, which we don't need to do here + // As a result, we manually sort the line styles to avoid that unnecessary processing + styles.lineStyles?.sort() + stylesReceiver?.setStyles(analyzer, styles) + } + + private fun processNextMessage() { + val message = messageChannel.take() + if (isDestroyed) { + return + } + + try { + when (message) { + is Init -> doInit(message) + is Mod -> doMod(message) + } + } catch (err: Throwable) { + val langName = languageSpec.language.name + val msgType = message.javaClass.simpleName + val msgTypeSuffix = + if (message is Mod) { + "[start=${message.data.start}, end=${message.data.end}, type=${if (message.data.changedText == null) "delete" else "insert"}]" + } else { + "" + } + val pendingMsgs = messageChannel.size + log.error( + "AnalyzeWorker[lang={}, message={}{}], pendingMsgs={}] crashed", + langName, + msgType, + msgTypeSuffix, + pendingMsgs, + err, + ) + } + } + + private fun doInit(init: Init) { + document.requestCancellationAndWaitIfParsing() + + check(!isInitialized) { + "'Init' must be the first message to TsAnalyzeWorker" + } + + document.doInit(init.data) + document.reparse() + updateStyles() + + isInitialized = true + } + + private fun doMod(mod: Mod) { + check(isInitialized) { + "'Init' must be the first message to TsAnalyzeWorker" + } + + val textMod = mod.data + val edit = textMod.edit + + val oldTree = tree!! + oldTree.edit(edit) + + document.doMod(textMod) + + (edit as? TreeSitterInputEdit?)?.recycle() + + document.requestCancellationAndWaitIfParsing() + + if (isDestroyed) { + return + } + + document.reparse(oldTree) + + oldTree.close() + updateStyles() + } + + private fun updateStyles() { + if (isDestroyed || messageChannel.isNotEmpty() || tree?.canAccess() != true) { + // analyzer stopped or + // more message need to be processed + return + } + + val tree = tree!! + val scopedVariables = TsScopedVariables(tree, text, languageSpec) + val oldSpans = (styles.spans as? LineSpansGenerator?) + val oldBrackets = analyzer.currentBracketPairs + + oldSpans?.destroy() + + // Use separate tree copies for the background worker and the UI thread + // to prevent concurrent access crashes. + styles.spans = + LineSpansGenerator( + tree.copy(), + reference.lineCount, + reference.reference, + theme, + languageSpec, + scopedVariables, + spanFactory, + requestRedraw = { stylesReceiver?.setStyles(analyzer, styles) }, + ) + + val newBrackets = TsBracketPairs(tree.copy(), languageSpec) + analyzer.currentBracketPairs = newBrackets + + val oldBlocks = styles.blocks + updateCodeBlocks() + oldBlocks?.also { ObjectAllocator.recycleBlockLines(it) } + + stylesReceiver?.setStyles(analyzer, styles) + stylesReceiver?.updateBracketProvider(analyzer, newBrackets) + + oldBrackets?.let { Handler(Looper.getMainLooper()).post { it.close() } } + } + + private fun updateCodeBlocks() { + if (languageSpec.blocksQuery.patternCount == 0 || + !languageSpec.blocksQuery.canAccess() || + tree?.canAccess() != true + ) { + return + } + + val blocks = mutableListOf() + TSQueryCursor.create().use { cursor -> + + cursor.safeExecQueryCursor( + query = languageSpec.blocksQuery, + tree = tree, + recycleNodeAfterUse = true, + matchCondition = { !isDestroyed }, + onClosedOrEdited = { blocks.clear() }, + debugName = "TsAnalyzeManager.updateCodeBlocks()", + ) { match -> + if (!languageSpec.blocksPredicator.doPredicate( + languageSpec.predicates, + text, + match, + ) + ) { + return@safeExecQueryCursor + } + + match.captures.forEach { capture -> + val block = ObjectAllocator.obtainBlockLine() + var node = capture.node + val start = node.startPoint + + block.startLine = start.row + block.startColumn = start.column / 2 + + val end = + if (languageSpec.blocksQuery + .getCaptureNameForId(capture.index) + .endsWith(".marked") + ) { + // Goto last terminal element + while (node.childCount > 0) { + node = node.getChild(node.childCount - 1) + } + node.startPoint + } else { + node.endPoint + } + block.endLine = end.row + block.endColumn = end.column / 2 + if (block.endLine - block.startLine > 1) { + blocks.add(block) + } + + (capture as? TreeSitterQueryCapture?)?.recycle() + } + } + } + + val distinct = blocks.asSequence().distinct().toMutableList() + styles.blocks = distinct + styles.finishBuilding() + } } internal interface Message { - - val data: T + val data: T } -internal data class Init(override val data: TextInit) : Message +internal data class Init( + override val data: TextInit, +) : Message -internal data class Mod(override val data: TextMod) : Message +internal data class Mod( + override val data: TextMod, +) : Message internal data class TextInit( - val text: String, - val contentVersion: Long + val text: String, + val contentVersion: Long, ) internal data class TextMod( - val start: Int, - val end: Int, - val edit: TSInputEdit, - val changedText: String?, - val contentVersion: Long -) \ No newline at end of file + val start: Int, + val end: Int, + val edit: TSInputEdit, + val changedText: String?, + val contentVersion: Long, +) From 66f3742c140a3951d82d0bdbcc85ae9d28f816f8 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 14:18:09 -0700 Subject: [PATCH 2/8] ADFA-5401: Quiesce the tree-sitter analyzer before freeing its natives Closing a project could SIGSEGV in ts_query_cursor_next_match. Two teardown paths freed native tree-sitter objects while other threads were still using them. Instrumenting a debug build and reproducing on device pinned the object down: it is the shared TSQuery. 14:06:31.578 TreeSitterWorker CAPTURE enter/exit query=189780455 14:06:31.978 main GEN.destroy 14:06:31.978 main SPEC.close ENTER query=189780455 LineSpansGenerator.captureRegion runs on the generator's own executor; TsLanguageSpec.close(), reached from EditorHandlerActivity.preDestroy via TSLanguageRegistry.destroy(), frees the query on the main thread. Nothing serialized the two, and captureRegion guards only tree.canAccess(), never the query. The generator already frees its own tree correctly, by queueing tree.close() onto the very executor its queries run on - the shared query just never got the same treatment. Two fixes, both making an existing intent actually hold: - LineSpansGenerator.destroy() already called tsExecutor.shutdown(); it now also awaits termination, so when it returns no capture can still be running. The call order (destroy before the spec closes) was already right; it simply was not waited on. - TsAnalyzeWorker.stop() cancelled the job and closed the dispatcher, but the loop parks in LinkedBlockingQueue.take(), which is a blocking call, not a suspension point: cancellation does not wake it, and closing the dispatcher just reroutes the continuation to Dispatchers.IO. It now hands the loop a Stop sentinel, joins it, and only then closes the document and the dispatcher. Both waits are bounded at 500 ms and log if they expire. Measured on device, the join completes in about 1 ms, so the timeout is a safety valve rather than a wait on the close path. Verified on device (Pixel 6 Pro, Android 17): four open/close cycles with the log panel open - the shape that produced the original crash, since the log view's analyzer churns while the project closes. No SIGSEGV, no "Cannot access native object", no timeout warnings, same pid throughout, and "Analyzer job completed" now lands 1 ms after "Stopping TsAnalyzeWorker" and before anything is freed. No unit test: editor-treesitter has no test source set, and adding one would mean new test dependencies in vendored sora-editor code. The argument here is the mechanism plus the measured ordering, not the absence of a crash in four cycles. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../sora/editor/ts/LineSpansGenerator.kt | 15 +++++++ .../rosemoe/sora/editor/ts/TsAnalyzeWorker.kt | 39 ++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt index 8517e78bff..996bc17af9 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt @@ -71,6 +71,7 @@ import kotlinx.coroutines.launch import java.util.TreeMap import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger @@ -95,6 +96,9 @@ class LineSpansGenerator( const val CACHE_THRESHOLD = 100 const val TAG = "LineSpansGenerator" + /** Upper bound on waiting for an in-flight query when tearing the generator down. */ + private const val SHUTDOWN_TIMEOUT_MS = 500L + /** * Delay in milliseconds to batch UI redraws, preventing frame drops * when rapidly calculating multiple lines. @@ -145,6 +149,17 @@ class LineSpansGenerator( tsExecutor.execute { runCatching { tree.close() } } tsExecutor.shutdown() + + // The shared TSQuery is freed on the main thread immediately after this returns + // (TSLanguageRegistry.destroy -> TsLanguageSpec.close), and captureRegion guards only the + // tree, not the query. Draining the executor first is what makes "no query is running" true + // rather than merely likely - a capture measures well under a millisecond, so the timeout is + // a safety valve, not a wait (ADFA-5401). + runCatching { + if (!tsExecutor.awaitTermination(SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + Log.w(TAG, "Tree-sitter query executor did not drain within $SHUTDOWN_TIMEOUT_MS ms") + } + }.onFailure { Thread.currentThread().interrupt() } } fun captureRegion( diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt index 6d74366c4f..6a4053e612 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt @@ -43,6 +43,8 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.newSingleThreadContext +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull import org.slf4j.LoggerFactory import java.util.concurrent.CancellationException import java.util.concurrent.LinkedBlockingQueue @@ -60,6 +62,9 @@ class TsAnalyzeWorker( ) { companion object { private val log = LoggerFactory.getLogger(TsAnalyzeWorker::class.java) + + /** Upper bound on waiting for the analyzer loop to finish before freeing its document. */ + private const val STOP_TIMEOUT_MS = 500L } var stylesReceiver: StyleReceiver? = null @@ -106,11 +111,32 @@ class TsAnalyzeWorker( document.requestCancellationAndWaitIfParsing() - analyzerContext.close() messageChannel.clear() analyzerJob?.cancel(CancellationException("Requested to be stopped")) + + // Cancelling does not wake a thread parked in take(), and closing the dispatcher does not stop + // it either - kotlinx reroutes the rejected dispatch to Dispatchers.IO. Hand the loop a message + // so it can see isDestroyed and return. + messageChannel.offer(Stop) + + // The document's native objects are freed just below, and the shared TSQuery right after this + // returns, so the loop must be *finished*, not merely cancelled. Anything still in doMod would + // be reading memory that is about to go away (ADFA-5401). + val stopped = + runBlocking { + withTimeoutOrNull(STOP_TIMEOUT_MS) { + analyzerJob?.join() + true + } + } != null + + if (!stopped) { + log.warn("Analyzer did not stop within {} ms; freeing its document anyway", STOP_TIMEOUT_MS) + } + analyzerScope.cancel(CancellationException("Requested to be stopped")) document.close() + analyzerContext.close() } fun start() { @@ -205,7 +231,7 @@ class TsAnalyzeWorker( private fun processNextMessage() { val message = messageChannel.take() - if (isDestroyed) { + if (isDestroyed || message is Stop) { return } @@ -395,6 +421,15 @@ internal data class Mod( override val data: TextMod, ) : Message +/** + * Wakes the worker's blocking [java.util.concurrent.LinkedBlockingQueue.take] so it can observe + * that it has been destroyed. Cancelling the job cannot do this: take() is a blocking call, not a + * suspension point (ADFA-5401). + */ +internal data object Stop : Message { + override val data = Unit +} + internal data class TextInit( val text: String, val contentVersion: Long, From eeb56365d1cda58866de4ca0a69382d1eee64233 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 15:47:31 -0700 Subject: [PATCH 3/8] style: spotless reformat of tsUtils, no functional change The ADFA-5401 rework adds a guard in this file, which enrolls it in the `ratchetFrom = origin/stage` ratchet and reformats it in full: it was 2-space indented and becomes tabs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../androidide/treesitter/api/tsUtils.kt | 258 +++++++++--------- 1 file changed, 129 insertions(+), 129 deletions(-) diff --git a/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt b/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt index bf98ada14b..c7499f7a77 100644 --- a/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt +++ b/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt @@ -35,53 +35,53 @@ internal val log = LoggerFactory.getLogger("TsUtilsKt") * This method does not close the [TSQueryCursor] instance. */ inline fun TSQueryCursor.safeExecQueryCursor( - query: TSQuery, - tree: TSTree?, - recycleNodeAfterUse: Boolean = true, - crossinline matchCondition: (TSQueryMatch?) -> Boolean = { true }, - crossinline whileTrue: (TSQueryMatch?) -> Boolean = { true }, - crossinline onClosedOrEdited: () -> Unit = {}, - debugName: String = "", - debugLogging: Boolean = false, - crossinline action: (TSQueryMatch) -> ResultT +query: TSQuery, +tree: TSTree?, +recycleNodeAfterUse: Boolean = true, +crossinline matchCondition: (TSQueryMatch?) -> Boolean = { true }, +crossinline whileTrue: (TSQueryMatch?) -> Boolean = { true }, +crossinline onClosedOrEdited: () -> Unit = {}, +debugName: String = "", +debugLogging: Boolean = false, +crossinline action: (TSQueryMatch) -> ResultT ): ResultT? { - if (tree == null || !tree.canAccess()) { - if (debugLogging) { - log.debug("$debugName: Cannot execute query, tree is null or not accessible", "tree=$tree", - "tree.canAccess=${tree?.canAccess()}") - } - return null - } - - val rootNode = tree.rootNode - if (!rootNode.canAccess() || rootNode.hasChanges()) { - if (debugLogging) { - log.debug( - "$debugName, Cannot execute query, tree's root node is not accessible or has been edited", - "rootNode=$rootNode", "rootNode.canAccess=${rootNode.canAccess()}", - "rootNode.hasChanges=${rootNode.canAccess() && rootNode.hasChanges()}") - } - return null - } - - return safeExecQueryCursor( - query = query, - node = rootNode, - recycleNodeAfterUse = recycleNodeAfterUse, - matchCondition = { - val result = tree.canAccess() && matchCondition(it) - if (!result && debugLogging) { - log.debug("$debugName: tree.canAccess=${tree.canAccess()}") - } - result - }, - whileTrue = whileTrue, - onClosedOrEdited = onClosedOrEdited, - debugName = debugName, - debugLogging = debugLogging, - action = action - ) +if (tree == null || !tree.canAccess()) { + if (debugLogging) { + log.debug("$debugName: Cannot execute query, tree is null or not accessible", "tree=$tree", + "tree.canAccess=${tree?.canAccess()}") + } + return null +} + +val rootNode = tree.rootNode +if (!rootNode.canAccess() || rootNode.hasChanges()) { + if (debugLogging) { + log.debug( + "$debugName, Cannot execute query, tree's root node is not accessible or has been edited", + "rootNode=$rootNode", "rootNode.canAccess=${rootNode.canAccess()}", + "rootNode.hasChanges=${rootNode.canAccess() && rootNode.hasChanges()}") + } + return null +} + +return safeExecQueryCursor( + query = query, + node = rootNode, + recycleNodeAfterUse = recycleNodeAfterUse, + matchCondition = { + val result = tree.canAccess() && matchCondition(it) + if (!result && debugLogging) { + log.debug("$debugName: tree.canAccess=${tree.canAccess()}") + } + result + }, + whileTrue = whileTrue, + onClosedOrEdited = onClosedOrEdited, + debugName = debugName, + debugLogging = debugLogging, + action = action +) } /** @@ -92,95 +92,95 @@ inline fun TSQueryCursor.safeExecQueryCursor( * This method does not close the [TSQueryCursor] instance. */ inline fun TSQueryCursor.safeExecQueryCursor( - query: TSQuery, - node: TSNode, - recycleNodeAfterUse: Boolean = true, - crossinline matchCondition: (TSQueryMatch?) -> Boolean = { true }, - crossinline whileTrue: (TSQueryMatch?) -> Boolean = { true }, - crossinline onClosedOrEdited: () -> Unit = {}, - debugName: String = "", - debugLogging: Boolean = false, - crossinline action: (TSQueryMatch) -> ResultT +query: TSQuery, +node: TSNode, +recycleNodeAfterUse: Boolean = true, +crossinline matchCondition: (TSQueryMatch?) -> Boolean = { true }, +crossinline whileTrue: (TSQueryMatch?) -> Boolean = { true }, +crossinline onClosedOrEdited: () -> Unit = {}, +debugName: String = "", +debugLogging: Boolean = false, +crossinline action: (TSQueryMatch) -> ResultT ): ResultT? { - return doSafeExecQueryCursor( - query = query, - node = node, - recycleNodeAfterUse = recycleNodeAfterUse, - matchCondition = { match -> - match != null && canAccess() && node.canAccess() && !node.hasChanges() && matchCondition( - match) - }, - whileTrue = whileTrue, - onClosedOrEdited = onClosedOrEdited, - debugName = debugName, - debugLogging = debugLogging, - action = action) +return doSafeExecQueryCursor( + query = query, + node = node, + recycleNodeAfterUse = recycleNodeAfterUse, + matchCondition = { match -> + match != null && canAccess() && node.canAccess() && !node.hasChanges() && matchCondition( + match) + }, + whileTrue = whileTrue, + onClosedOrEdited = onClosedOrEdited, + debugName = debugName, + debugLogging = debugLogging, + action = action) } @PublishedApi internal inline fun TSQueryCursor.doSafeExecQueryCursor( - query: TSQuery, - node: TSNode, - recycleNodeAfterUse: Boolean = true, - crossinline matchCondition: (TSQueryMatch?) -> Boolean, - crossinline whileTrue: (TSQueryMatch?) -> Boolean, - crossinline onClosedOrEdited: () -> Unit, - debugName: String = "", - debugLogging: Boolean = false, - crossinline action: (TSQueryMatch) -> ResultT +query: TSQuery, +node: TSNode, +recycleNodeAfterUse: Boolean = true, +crossinline matchCondition: (TSQueryMatch?) -> Boolean, +crossinline whileTrue: (TSQueryMatch?) -> Boolean, +crossinline onClosedOrEdited: () -> Unit, +debugName: String = "", +debugLogging: Boolean = false, +crossinline action: (TSQueryMatch) -> ResultT ): ResultT? { - if (!query.canAccess()) { - if (debugLogging) { - log.debug("$debugName: Cannot execute query, query is not accessible") - } - return null - } - - if (!node.canAccess() || node.hasChanges()) { - if (debugLogging) { - log.debug("$debugName: Cannot execute query, node is not accessible or has been edited", - "node.canAccess=${node.canAccess()}", - "node.hasChanges=${node.canAccess() && node.hasChanges()}") - } - return null - } - - exec(query, node) - var match = nextMatch() - while (matchCondition(match) && whileTrue(match)) { - - val result = action(match) - - if (!matchCondition(match)) { - if (debugLogging) { - log.debug( - "$debugName: Cannot proceed with query operation.", - "cursor.canAccess=${canAccess()}", - "query.canAccess=${query.canAccess()}", - "node.canAccess=${node.canAccess()}", - "node.hasChanges=${node.canAccess() && node.hasErrors()}" - ) - } - onClosedOrEdited() - break - } - - (match as? TreeSitterQueryMatch?)?.recycle() - - // if the action does not produce any output and simply returns Unit (void) - // then ignore the result and continue with the capture - if (result != Unit && result != null) { - return result - } - - match = nextMatch() - } - - if (recycleNodeAfterUse && node is TreeSitterNode && !node.isRecycled) { - node.recycle() - } - - return null -} \ No newline at end of file +if (!query.canAccess()) { + if (debugLogging) { + log.debug("$debugName: Cannot execute query, query is not accessible") + } + return null +} + +if (!node.canAccess() || node.hasChanges()) { + if (debugLogging) { + log.debug("$debugName: Cannot execute query, node is not accessible or has been edited", + "node.canAccess=${node.canAccess()}", + "node.hasChanges=${node.canAccess() && node.hasChanges()}") + } + return null +} + +exec(query, node) +var match = nextMatch() +while (matchCondition(match) && whileTrue(match)) { + + val result = action(match) + + if (!matchCondition(match)) { + if (debugLogging) { + log.debug( + "$debugName: Cannot proceed with query operation.", + "cursor.canAccess=${canAccess()}", + "query.canAccess=${query.canAccess()}", + "node.canAccess=${node.canAccess()}", + "node.hasChanges=${node.canAccess() && node.hasErrors()}" + ) + } + onClosedOrEdited() + break + } + + (match as? TreeSitterQueryMatch?)?.recycle() + + // if the action does not produce any output and simply returns Unit (void) + // then ignore the result and continue with the capture + if (result != Unit && result != null) { + return result + } + + match = nextMatch() +} + +if (recycleNodeAfterUse && node is TreeSitterNode && !node.isRecycled) { + node.recycle() +} + +return null +} From 98c2acbf8db30978807142c602461314848b29c3 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 16:02:47 -0700 Subject: [PATCH 4/8] ADFA-5401: Guard the shared query instead of blocking on teardown Replaces the previous approach in this branch, which was wrong. Review established three things I had not checked: - TsAnalyzeManager.reset() calls rerun(), which calls stop(), and CodeEditor.setText() calls reset(). So stop() is on the hot path - every log-filter change, build-output filter change and tab open - not the teardown path I assumed. A 500 ms runBlocking join there freezes the main thread while the user types in a filter box. - updateStyles() calls oldSpans?.destroy() after every reparse, so the awaitTermination added there was also per-keystroke, on the analyzer thread, and nested inside stop()'s join rather than composing with it. - On timeout, stop() logged and then freed the document anyway - doing exactly the unsafe thing the change existed to prevent. The blocking is gone. Instead the guard goes where the crash actually happens: doSafeExecQueryCursor checked query.canAccess() once before the loop but never inside it, so a query freed mid-loop made the next nextMatch() dereference a dangling TSQuery. Adding it to the per-iteration matchCondition covers every caller - LineSpansGenerator.captureRegion, updateCodeBlocks, TsScopedVariables.init and TsBracketPairs - at the mechanism, at no cost on any path. Also kept from the first attempt, both cheap and both still right: - The Stop sentinel, so the loop leaves its blocking take() promptly. Cancelling cannot wake it and closing the dispatcher only reroutes the continuation to Dispatchers.IO. - analyzerContext.close() last, after the document. - isDestroyed is now @Volatile. It is written by whoever calls stop() and read by the analyzer loop and the query guards; the Stop message was the only happens-before edge. This narrows the window rather than closing it: canAccess() is still check-then-use, and a free landing between the check and nextMatch() would still crash. Closing it completely needs ownership or refcounting on the native handles, which is a bigger change than this ticket. The guard matches the defensive idiom the library already uses everywhere else, and is strictly better than a main-thread wait that then frees regardless. The license header of LineSpansGenerator.kt is restored to two separate blocks - merging AndroidIDE's GPL-3 notice and Rosemoe's LGPL-2.1 notice into one block behind a divider blurred two distinct grants on vendored code, and the divider itself is the kind of decorative separator CLAUDE.md forbids. The two ktlint rules it trips are suppressed at file level instead. Verified on device (Pixel 6 Pro, Android 17): four open/close cycles with the log panel open, same pid throughout, no SIGSEGV and no "Cannot access native object". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../androidide/treesitter/api/tsUtils.kt | 8 +++-- .../sora/editor/ts/LineSpansGenerator.kt | 33 ++++++------------- .../rosemoe/sora/editor/ts/TsAnalyzeWorker.kt | 21 +++--------- 3 files changed, 20 insertions(+), 42 deletions(-) diff --git a/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt b/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt index c7499f7a77..e8f249447c 100644 --- a/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt +++ b/editor-api/src/main/java/com/itsaky/androidide/treesitter/api/tsUtils.kt @@ -108,8 +108,12 @@ return doSafeExecQueryCursor( node = node, recycleNodeAfterUse = recycleNodeAfterUse, matchCondition = { match -> - match != null && canAccess() && node.canAccess() && !node.hasChanges() && matchCondition( - match) + // query.canAccess() belongs here, not only in the pre-loop check below: the query is shared + // between the analyzer, the span generator and the bracket matcher, and whoever frees it does + // so on another thread. Without it, a free landing mid-loop makes the next nextMatch() + // dereference a dangling TSQuery and take the process down (ADFA-5401). + match != null && canAccess() && query.canAccess() && node.canAccess() && !node.hasChanges() && + matchCondition(match) }, whileTrue = whileTrue, onClosedOrEdited = onClosedOrEdited, diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt index 996bc17af9..a09cee43d2 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt @@ -13,9 +13,9 @@ * * You should have received a copy of the GNU General Public License * along with AndroidIDE. If not, see . - * - * ----------------------------------------------------------------------------- - * + */ + +/******************************************************************************* * sora-editor - the awesome code editor for Android * https://github.com/Rosemoe/sora-editor * Copyright (C) 2020-2023 Rosemoe @@ -37,7 +37,9 @@ * * Please contact Rosemoe by email 2073412493@qq.com if you need * additional information or have any questions - */ + ******************************************************************************/ + +@file:Suppress("ktlint:standard:kdoc", "ktlint:standard:no-consecutive-comments") package io.github.rosemoe.sora.editor.ts @@ -71,7 +73,6 @@ import kotlinx.coroutines.launch import java.util.TreeMap import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger @@ -96,9 +97,6 @@ class LineSpansGenerator( const val CACHE_THRESHOLD = 100 const val TAG = "LineSpansGenerator" - /** Upper bound on waiting for an in-flight query when tearing the generator down. */ - private const val SHUTDOWN_TIMEOUT_MS = 500L - /** * Delay in milliseconds to batch UI redraws, preventing frame drops * when rapidly calculating multiple lines. @@ -136,10 +134,10 @@ class LineSpansGenerator( } } -/** -* Queues the native tree destruction in the background -* so it doesn't close while a query is running. -*/ + /** + * Queues the native tree destruction in the background + * so it doesn't close while a query is running. + */ fun destroy() { scope.cancel() caches.evictAll() @@ -149,17 +147,6 @@ class LineSpansGenerator( tsExecutor.execute { runCatching { tree.close() } } tsExecutor.shutdown() - - // The shared TSQuery is freed on the main thread immediately after this returns - // (TSLanguageRegistry.destroy -> TsLanguageSpec.close), and captureRegion guards only the - // tree, not the query. Draining the executor first is what makes "no query is running" true - // rather than merely likely - a capture measures well under a millisecond, so the timeout is - // a safety valve, not a wait (ADFA-5401). - runCatching { - if (!tsExecutor.awaitTermination(SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { - Log.w(TAG, "Tree-sitter query executor did not drain within $SHUTDOWN_TIMEOUT_MS ms") - } - }.onFailure { Thread.currentThread().interrupt() } } fun captureRegion( diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt index 6a4053e612..de765a516e 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt @@ -43,8 +43,6 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.newSingleThreadContext -import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.withTimeoutOrNull import org.slf4j.LoggerFactory import java.util.concurrent.CancellationException import java.util.concurrent.LinkedBlockingQueue @@ -77,6 +75,10 @@ class TsAnalyzeWorker( private var analyzerJob: Job? = null private var isInitialized = false + + // Written by whoever calls stop(), read by the analyzer loop and by the query guards it + // passes as matchCondition; the only happens-before edge otherwise is the Stop message. + @Volatile private var isDestroyed = false val document = TsTextDocument(languageSpec.language) @@ -119,21 +121,6 @@ class TsAnalyzeWorker( // so it can see isDestroyed and return. messageChannel.offer(Stop) - // The document's native objects are freed just below, and the shared TSQuery right after this - // returns, so the loop must be *finished*, not merely cancelled. Anything still in doMod would - // be reading memory that is about to go away (ADFA-5401). - val stopped = - runBlocking { - withTimeoutOrNull(STOP_TIMEOUT_MS) { - analyzerJob?.join() - true - } - } != null - - if (!stopped) { - log.warn("Analyzer did not stop within {} ms; freeing its document anyway", STOP_TIMEOUT_MS) - } - analyzerScope.cancel(CancellationException("Requested to be stopped")) document.close() analyzerContext.close() From 61fa2ab930ab7723b6183eb4ea6a0871900ca198 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 16:31:40 -0700 Subject: [PATCH 5/8] ADFA-5401: Let the analyzer loop free its own natives; review fixes Review found that reverting the blocking join left the document path uncovered: stop() cancelled the loop and then immediately closed the document, so a worker inside doMod() or updateStyles() could still be reading the text and tree being freed. The Stop sentinel only unblocks a thread parked in take(); it does nothing for one that is busy. Fixed by moving the teardown into the loop's own finally, so the natives are released by the thread that uses them, once it has actually exited. That gets the ordering guarantee with no caller-side wait, which matters because stop() is on the setText/reset hot path. It is the same idiom LineSpansGenerator already uses for its tree. If start() was never called there is no loop to do it, so stop() closes them itself in that case, and closeNatives() closes the dispatcher in a finally so a throw from document.close() cannot strand the thread. Also from review: - stop() cleared the message queue, dropping pooled TreeSitterInputEdit instances that only doMod() recycles. Since stop() runs on every reset, that churn was per-interaction. It now drains and recycles. - STOP_TIMEOUT_MS was left behind by the reverted blocking attempt - referenced nowhere, with a KDoc describing a wait that no longer exists. Removed. - The file-level ktlint suppression on LineSpansGenerator covered standard:kdoc, which disabled KDoc checks for the whole file and hid that the reformat had left fifteen KDoc blocks flush at column 0 inside a tab-indented body. The suppression is now narrowed to the one rule the two-block licence header actually trips, and the blocks are reindented. - Two ASCII banner separators survived the reformat in the same file whose description cites CLAUDE.md's no-separator rule. The prose they wrapped is now KDoc on applyDecorations(). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../sora/editor/ts/LineSpansGenerator.kt | 82 +++++++++---------- .../rosemoe/sora/editor/ts/TsAnalyzeWorker.kt | 54 ++++++++++-- 2 files changed, 88 insertions(+), 48 deletions(-) diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt index a09cee43d2..7379f4e764 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt @@ -15,7 +15,7 @@ * along with AndroidIDE. If not, see . */ -/******************************************************************************* +/* ***************************************************************************** * sora-editor - the awesome code editor for Android * https://github.com/Rosemoe/sora-editor * Copyright (C) 2020-2023 Rosemoe @@ -39,7 +39,9 @@ * additional information or have any questions ******************************************************************************/ -@file:Suppress("ktlint:standard:kdoc", "ktlint:standard:no-consecutive-comments") +// Two licence grants: AndroidIDE's GPL-3 and, below it, sora-editor's LGPL-2.1. They stay +// separate blocks so neither is framed as part of the other; that trips one ktlint rule. +@file:Suppress("ktlint:standard:no-consecutive-comments") package io.github.rosemoe.sora.editor.ts @@ -104,10 +106,10 @@ class LineSpansGenerator( const val REDRAW_DEBOUNCE_DELAY_MS = 32L } -/** -* Thread-safe cache for calculated line spans. -* Automatically evicts the least recently used lines. -*/ + /** + * Thread-safe cache for calculated line spans. + * Automatically evicts the least recently used lines. + */ private val caches = LruCache>(CACHE_THRESHOLD) private val calculatingLines = ConcurrentHashMap.newKeySet() @@ -118,10 +120,10 @@ class LineSpansGenerator( private val tsDispatcher = tsExecutor.asCoroutineDispatcher() private val scope = CoroutineScope(SupervisorJob() + tsDispatcher) -/** -* Tracks content changes so the worker can instantly abort -* outdated calculations when the user types. -*/ + /** + * Tracks content changes so the worker can instantly abort + * outdated calculations when the user types. + */ private val contentVersion = AtomicInteger(0) private val mainHandler = Handler(Looper.getMainLooper()) private var isRefreshScheduled = AtomicBoolean(false) @@ -238,16 +240,14 @@ class LineSpansGenerator( return applyDecorations(list, startIndex, endIndex) } -// --------------------------------------------------------------------------- -// Plugin editor decorations -// -// After the base (tree-sitter) spans for a region are built, every registered -// EditorDecorationProvider is given the region and returns additive, foreground-only color -// spans, which are merged in here. The IDE is feature-agnostic — it knows nothing about what a -// provider decorates (brackets, indent guides, markers, ...). Providers run on this analyze -// thread and receive the full document content so they can use context outside the region. -// --------------------------------------------------------------------------- - + /** + * Merges plugin editor decorations into the base tree-sitter spans for a region. + * + * Every registered EditorDecorationProvider is given the region and returns additive, + * foreground-only colour spans. The IDE is feature-agnostic - it knows nothing about what a + * provider decorates (brackets, indent guides, markers, ...). Providers run on this analyze + * thread and receive the full document content, so they can use context outside the region. + */ private fun applyDecorations( list: MutableList, startIndex: Int, @@ -274,12 +274,12 @@ class LineSpansGenerator( return mergeDecorations(list, startIndex, endIndex, decos) } -/** -* Merges additive, foreground-only decoration spans into [list]: overrides only the foreground -* color of the covered characters while preserving every base style underneath, and keeps the -* strictly-ascending, non-overlapping span ordering the renderer requires. Decoration offsets are -* absolute; they are clipped to the region and converted to line-relative columns. -*/ + /** + * Merges additive, foreground-only decoration spans into [list]: overrides only the foreground + * color of the covered characters while preserving every base style underneath, and keeps the + * strictly-ascending, non-overlapping span ordering the renderer requires. Decoration offsets are + * absolute; they are clipped to the region and converted to line-relative columns. + */ private fun mergeDecorations( list: MutableList, startIndex: Int, @@ -411,13 +411,13 @@ class LineSpansGenerator( } } -/** -* Shifts span columns horizontally to prevent visual flickering during inline edits. -* -* @param line Line index of the modification. -* @param startColumn Column index where the shift begins. -* @param colDiff Number of columns to shift. -*/ + /** + * Shifts span columns horizontally to prevent visual flickering during inline edits. + * + * @param line Line index of the modification. + * @param startColumn Column index where the shift begins. + * @param colDiff Number of columns to shift. + */ private fun shiftSpansOnLine( line: Int, startColumn: Int, @@ -430,11 +430,11 @@ class LineSpansGenerator( } } -/** -* Rebuilds the line cache for vertical text shifts (line additions or deletions). -* -* @param action Logic to determine how each cached line is re-inserted. -*/ + /** + * Rebuilds the line cache for vertical text shifts (line additions or deletions). + * + * @param action Logic to determine how each cached line is re-inserted. + */ private inline fun rebuildCache(action: (line: Int, spans: MutableList, cache: LruCache>) -> Unit) { val snapshot = caches.snapshot() caches.evictAll() @@ -491,9 +491,9 @@ class LineSpansGenerator( } } -/** -* Groups redraw requests together to avoid overloading the UI thread. -*/ + /** + * Groups redraw requests together to avoid overloading the UI thread. + */ private fun scheduleRefresh() { if (!isRefreshScheduled.compareAndSet(false, true)) return diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt index de765a516e..e178106e81 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt @@ -113,17 +113,48 @@ class TsAnalyzeWorker( document.requestCancellationAndWaitIfParsing() - messageChannel.clear() - analyzerJob?.cancel(CancellationException("Requested to be stopped")) + drainPendingMessages() + + val job = analyzerJob + job?.cancel(CancellationException("Requested to be stopped")) // Cancelling does not wake a thread parked in take(), and closing the dispatcher does not stop // it either - kotlinx reroutes the rejected dispatch to Dispatchers.IO. Hand the loop a message - // so it can see isDestroyed and return. + // so it can see isDestroyed and return; it frees the natives on its way out. messageChannel.offer(Stop) analyzerScope.cancel(CancellationException("Requested to be stopped")) - document.close() - analyzerContext.close() + + if (job == null) { + // start() was never called, so there is no loop to do it. + closeNatives() + } + } + + /** + * Releases the document and the dispatcher. Runs on the analyzer thread once its loop has + * finished, so nothing can still be reading what it frees. + */ + private fun closeNatives() { + try { + document.close() + } finally { + analyzerContext.close() + } + } + + /** + * Empties the queue, recycling each message's pooled natives. [LinkedBlockingQueue.clear] would + * drop them without recycling, and stop() runs on every reset - so that churn would be + * per-interaction, not per-close. + */ + private fun drainPendingMessages() { + while (true) { + when (val pending = messageChannel.poll() ?: return) { + is Mod -> (pending.data.edit as? TreeSitterInputEdit?)?.recycle() + else -> Unit + } + } } fun start() { @@ -132,8 +163,17 @@ class TsAnalyzeWorker( analyzerJob = analyzerScope .launch { - while (!isDestroyed && isActive) { - processNextMessage() + try { + while (!isDestroyed && isActive) { + processNextMessage() + } + } finally { + // Freed here, by the thread that uses them, rather than by whoever calls + // stop(): the loop can be mid doMod()/updateStyles() when stop() runs, and a + // caller-side close would pull the text and tree out from under it. Same idiom + // as LineSpansGenerator, which posts its own tree.close() onto its executor. + // stop() cannot wait for this instead - it is on the setText/reset hot path. + closeNatives() } }.also { job -> job.invokeOnCompletion { error -> From ae196e33dd1e16ae6c48fc4e1ec6646aff7b66fa Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 16:32:00 -0700 Subject: [PATCH 6/8] style: spotless reformat of TreeSitterIndentProvider, no functional change The ADFA-5401 follow-up adds a query guard here, which enrolls the file in the `ratchetFrom = origin/stage` ratchet and reformats it in full: it was 2-space indented (with one stray tab-indented line) and becomes tabs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../treesitter/TreeSitterIndentProvider.kt | 983 +++++++++--------- 1 file changed, 519 insertions(+), 464 deletions(-) diff --git a/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt b/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt index 0bf7e9f6e2..f9a981d9eb 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt @@ -50,128 +50,132 @@ import kotlin.math.min * @author Akash Yadav */ class TreeSitterIndentProvider( - private val languageSpec: TreeSitterLanguageSpec, - private val analyzer: TsAnalyzeWorker, - private val indentSize: Int + private val languageSpec: TreeSitterLanguageSpec, + private val analyzer: TsAnalyzeWorker, + private val indentSize: Int, ) { + companion object { + private const val IDENT_AUTO = "indent.auto" + private const val IDENT_BEGIN = "indent.begin" + private const val IDENT_END = "indent.end" + private const val IDENT_DEDENT = "indent.dedent" + private const val IDENT_BRANCH = "indent.branch" + private const val IDENT_IGNORE = "indent.ignore" + private const val IDENT_ALIGN = "indent.align" + private const val IDENT_ZERO = "indent.zero" + + private const val IDENT_TYP_COUNT = 8 // increment this when adding a new indent type above + + private val log = LoggerFactory.getLogger(TreeSitterIndentProvider::class.java) + internal const val INDENTATION_ERR = Int.MIN_VALUE + internal const val INDENT_ALIGN_ZERO = Int.MIN_VALUE + internal const val INDENT_AUTO = Int.MAX_VALUE + + private val DELIMITER_REGEX = Regex("""[\-.+\[\]()$^\\?*]""") + private const val CONTEXT_LINES_LIMIT = 5 + } - companion object { - - private const val IDENT_AUTO = "indent.auto" - private const val IDENT_BEGIN = "indent.begin" - private const val IDENT_END = "indent.end" - private const val IDENT_DEDENT = "indent.dedent" - private const val IDENT_BRANCH = "indent.branch" - private const val IDENT_IGNORE = "indent.ignore" - private const val IDENT_ALIGN = "indent.align" - private const val IDENT_ZERO = "indent.zero" - - private const val IDENT_TYP_COUNT = 8 // increment this when adding a new indent type above - - private val log = LoggerFactory.getLogger(TreeSitterIndentProvider::class.java) - internal const val INDENTATION_ERR = Int.MIN_VALUE - internal const val INDENT_ALIGN_ZERO = Int.MIN_VALUE - internal const val INDENT_AUTO = Int.MAX_VALUE - - private val DELIMITER_REGEX = Regex("""[\-.+\[\]()$^\\?*]""") - private const val CONTEXT_LINES_LIMIT = 5 - } - - fun getIndentsForLines( - content: Content, - positions: LongArray, - default: Int = INDENTATION_ERR - ): IntArray { - log.debug("getIndentsForLine(Content({}),{})", content.length, - positions.joinToString(",") { "${IntPair.getFirst(it)}:${IntPair.getSecond(it)}" }) - val defaultIndents = IntArray(positions.size) { default } - - // not really needed, but just in case - if (content.isEmpty() || positions.isEmpty()) { - return defaultIndents - } - - val document = analyzer.document - TSParser.create().use { parser -> - parser.language = document.parser.language - - var closeTree = true - val tree = if (content.documentVersion == document.version) { - // avoid converting the content to string if not really needed - log.info("Re-using cached tree from document version {}", document.version) - closeTree = false - document.tree - } else { - log.info( - "Re-parsing content for indentation as document version {} does not match version {}", - document.version, - content.documentVersion - ) - - (document.tree?.copy() ?: return defaultIndents).use { copiedTree -> - parser.parseString(copiedTree, content.toString()) - } - } - - if (tree == null) { - log.info("Parsed tree is null, returning default indent: {}", default) - return defaultIndents - } - - try { - return computeIndents(tree, content, positions, defaultIndents) - .also { indents -> - log.debug("Computed indents: {}", indents.joinToString(",")) - } - } finally { - if (closeTree) { - tree.close() - } - } - } - } - - private fun computeIndents( - tree: TSTree, - content: Content, - positions: LongArray, - defaultIndents: IntArray - ): IntArray { - val indentsQuery = languageSpec.indentsQuery ?: run { - log.info("Cannot compute indents. Indents query is null.") - return defaultIndents - } - - if (indentsQuery == TSQuery.EMPTY) { - log.info("Cannot compute indents. Indents query is empty.") - return defaultIndents - } - - val rootNode = tree.rootNode ?: run { - log.info("Cannot compute indents. Root node is null.") - return defaultIndents - } - - return TSQueryCursor.create().use { cursor -> - cursor.addPredicateHandler(SetDirectiveHandler()) + fun getIndentsForLines( + content: Content, + positions: LongArray, + default: Int = INDENTATION_ERR, + ): IntArray { + log.debug( + "getIndentsForLine(Content({}),{})", + content.length, + positions.joinToString(",") { "${IntPair.getFirst(it)}:${IntPair.getSecond(it)}" }, + ) + val defaultIndents = IntArray(positions.size) { default } + + // not really needed, but just in case + if (content.isEmpty() || positions.isEmpty()) { + return defaultIndents + } + + val document = analyzer.document + TSParser.create().use { parser -> + parser.language = document.parser.language + + var closeTree = true + val tree = + if (content.documentVersion == document.version) { + // avoid converting the content to string if not really needed + log.info("Re-using cached tree from document version {}", document.version) + closeTree = false + document.tree + } else { + log.info( + "Re-parsing content for indentation as document version {} does not match version {}", + document.version, + content.documentVersion, + ) + + (document.tree?.copy() ?: return defaultIndents).use { copiedTree -> + parser.parseString(copiedTree, content.toString()) + } + } + + if (tree == null) { + log.info("Parsed tree is null, returning default indent: {}", default) + return defaultIndents + } + + try { + return computeIndents(tree, content, positions, defaultIndents) + .also { indents -> + log.debug("Computed indents: {}", indents.joinToString(",")) + } + } finally { + if (closeTree) { + tree.close() + } + } + } + } + + private fun computeIndents( + tree: TSTree, + content: Content, + positions: LongArray, + defaultIndents: IntArray, + ): IntArray { + val indentsQuery = + languageSpec.indentsQuery ?: run { + log.info("Cannot compute indents. Indents query is null.") + return defaultIndents + } + + if (indentsQuery == TSQuery.EMPTY) { + log.info("Cannot compute indents. Indents query is empty.") + return defaultIndents + } + + val rootNode = + tree.rootNode ?: run { + log.info("Cannot compute indents. Root node is null.") + return defaultIndents + } + + return TSQueryCursor.create().use { cursor -> + cursor.addPredicateHandler(SetDirectiveHandler()) optimizeCursorRange(positions, content, cursor) - cursor.exec(indentsQuery, tree.rootNode) + cursor.exec(indentsQuery, tree.rootNode) - val indents = getIndents(languageSpec.indentsQuery, cursor) - return@use IntArray(positions.size) { index -> - val line = IntPair.getFirst(positions[index]) - val column = IntPair.getSecond(positions[index]) - computeIndentForLine(content, line, column, defaultIndents[index], rootNode, indents) - } - } - } + val indents = getIndents(languageSpec.indentsQuery, cursor) + return@use IntArray(positions.size) { index -> + val line = IntPair.getFirst(positions[index]) + val column = IntPair.getSecond(positions[index]) + computeIndentForLine(content, line, column, defaultIndents[index], rootNode, indents) + } + } + } private fun optimizeCursorRange( positions: LongArray, content: Content, - cursor: TSQueryCursor? + cursor: TSQueryCursor?, ) { if (!positions.isNotEmpty()) return @@ -185,363 +189,414 @@ class TreeSitterIndentProvider( cursor?.setByteRange(startByte, endByte) } - private fun computeIndentForLine( - content: Content, - line: Int, - column: Int, - default: Int, - rootNode: TSNode, - indents: IndentsContainer - ): Int { - val isEmptyLine = content.getLine(line).trimmedLength() == 0 - var node: TSNode? - - if (isEmptyLine) { - val prevlnum = content.previousNonBlankLine(line) - if (prevlnum == -1) { - log.error("Cannot compute indents. Unable to get previous non-blank line.") - return default - } else { - log.debug("Previous non-blank line: {}", prevlnum) - } - - var prevline: CharSequence = content.getLine(prevlnum) - val indentBytes = TextUtils.countLeadingSpaceCount(prevline, indentSize) shl 1 - prevline = prevline.trim() - - // The final position can be trailing spaces, which should not affect indentation - node = content.getLastNodeAtLine(rootNode, prevlnum, - (indentBytes + prevline.length shl 1) - 2 - ) ?: run { - log.error("Unable to get last node at line: {}", prevlnum) - return default - } - - // TODO(itsaky): Make this an API - // Language defs must be able to specify captures which represent a comment - if (node.type == "comment") { - // The final node we capture of the previous line can be a comment node, which should also be ignored - // Unless the last line is an entire line of comment, ignore the comment range and find the last node again - val firstNode = content.getFirstNodeAtLine(rootNode, prevlnum, indentBytes) - val scol = node.startPoint.column - if (firstNode?.nodeId != node.nodeId) { - // In case the last captured node is a trailing comment node, re-trim the string - prevline = prevline.subSequence(0, (scol shr 1) - (indentBytes shr 1)).trim() - val col = indentBytes + ((prevline.length - 1) shl 1) - - node = content.getLastNodeAtLine(rootNode, prevlnum, col) - } - } - - if (indents[IDENT_END]!![node?.nodeId ?: 0] != null) { - node = content.getFirstNodeAtLine(rootNode, line) - } - } else { - node = content.getFirstNodeAtLine(rootNode, line, column shl 1) - } - - if (node == null || !node.canAccess()) { - log.error( - "Cannot compute indents. Unable to get node at line: {}. node={} node.canAccess={}", line, - node, node?.canAccess()) - return default - } - - var indent = 0 - - if (indents[IDENT_ZERO]?.containsKey(node.nodeId) == true) { - // indent.zero: align the node to the start of the line - log.debug("Zero indent for node: {}", node) - return INDENT_ALIGN_ZERO - } - - // map to store whether a given line is already processed - // this is to ensure that we do not accidentally apply multiple indent levels to the same line - val processedLines = mutableIntObjectMapOf() - - while (node != null && node.canAccess()) { - - val srow = node.startPoint.line - val erow = node.endPoint.line - - // do 'auto indent' if not marked as '@indent' - if (!indents.hasNode(IDENT_BEGIN, node) - && !indents.hasNode(IDENT_ALIGN, node) - && indents.hasNode(IDENT_AUTO, node) - && srow < line - && line <= erow - ) { - log.debug("Auto indent for node: {}", node) - return INDENT_AUTO - } - - // Do not indent if we are inside an @ignore block. - // If a node spans from L1,C1 to L2,C2, we know that lines where L1 < line <= L2 would - // have their indentations contained by the node. - if (!indents.hasNode(IDENT_BEGIN, node) - && indents.hasNode(IDENT_IGNORE, node) - && srow < line - && line <= erow - ) { - log.debug("Ignore indent for node: {}", node) - return default - } - - var isProcessed = false - - if (!processedLines.containsKey(srow) - && ((indents.hasNode(IDENT_BRANCH, node) && srow == line) - || (indents.hasNode(IDENT_DEDENT, node) && srow != line)) - ) { - indent -= indentSize - isProcessed = true - } - - // do not indent for nodes that starts-and-ends on same line and starts on target line (lnum) - val shouldProcess = !processedLines.containsKey(srow) - var isInError = false - if (shouldProcess) { - isInError = node.parent?.let { it.canAccess() && it.hasErrors() } == true - } - - if (shouldProcess && - (indents.hasNode(IDENT_BEGIN, node) - && (srow != erow || isInError || indents.hasMeta(IDENT_BEGIN, node, - "indent.immediate")) - && (srow != line || indents.hasMeta(IDENT_BEGIN, node, "indent.start_at_same_line"))) - ) { - indent += indentSize - isProcessed = true - } - - if (isInError && !indents.hasNode(IDENT_ALIGN, node)) { - // only when the node is in error, promote the - // first child's aligned indent to the error node - // to work around ((ERROR "X" . (_)) @aligned_indent (#set! "delimiter" "AB")) - // matching for all X, instead set do - // (ERROR "X" @aligned_indent (#set! "delimiter" "AB") . (_)) - // and we will fish it out here. - - for (i in 0 until node.childCount) { - val child = node.getChild(i) - if (indents.hasNode(IDENT_ALIGN, child)) { - indents[IDENT_ALIGN]!![node.nodeId] = indents[IDENT_ALIGN]!![child.nodeId]!! - break - } - } - } - - // do not indent for nodes that starts-and-ends on same line and starts on target line (lnum) - if (shouldProcess - && indents.hasNode(IDENT_ALIGN, node) - && (srow != erow || isInError) - && (srow != line) - ) { - val meta = indents.getMeta(IDENT_ALIGN, node)!! - var oDelimNode: TSNode? - var oIsLastInLine = false - var cDelimNode: TSNode? - var cIsLastInLine = false - var indentIsAbsolute = false - - if (meta.containsKey("indent.open_delimiter")) { - val r = findDelimiter(content, node, meta["indent.open_delimiter"]!!) - oDelimNode = r.first - oIsLastInLine = r.second - } else { - oDelimNode = node - } - - if (meta.containsKey("indent.close_delimiter")) { - val r = findDelimiter(content, node, meta["indent.close_delimiter"]!!) - cDelimNode = r.first - cIsLastInLine = r.second - } else { - cDelimNode = node - } - - if (oDelimNode != null) { - val osrow = oDelimNode.startPoint.row - val oscol = oDelimNode.startPoint.column - var csrow: Int? = null - if (cDelimNode != null) { - csrow = cDelimNode.startPoint.row - } - - if (oIsLastInLine) { - // hanging indent (previous line ended with starting delimiter) - // should be processed like indent - if (shouldProcess) { - indent += indentSize - if (cIsLastInLine) { - // If current line is outside the range of a node marked with `@aligned_indent` - // Then its indent level shouldn't be affected by `@aligned_indent` node - if (csrow != null && csrow < line) { - indent = max(indent - indentSize, 0) - } - } - } - } else { - // aligned indent - if (cIsLastInLine && csrow != null && osrow != csrow && csrow < line) { - // If current line is outside the range of a node marked with `@aligned_indent` - // Then its indent level shouldn't be affected by `@aligned_indent` node - indent = max(indent - indentSize, 0) - } else { - indent = oscol + (meta.getInt("indent.increment") ?: 1) - indentIsAbsolute = true - } - } - - // deal with final line - var avoidLastMatchingNext = false - if (csrow != null && csrow != osrow && csrow == line) { - // delims end on current line, and are not open and closed same line. - // then this last line may need additional indent to avoid clashes - // with the next. `indent.avoid_last_matching_next` controls this behavior, - // for example this is needed for function parameters. - - avoidLastMatchingNext = meta.getBolean("indent.avoid_last_matching_next") - ?: false - } - - if (avoidLastMatchingNext) { - // last line must be indented more in cases where - // it would be same indent as next line (we determine this as one - // width more than the open indent to avoid confusing with any - // hanging indents) - val osrowIndent = TextUtils.countLeadingSpaceCount(content.getLine(osrow), indentSize) - if (indent <= osrowIndent + indentSize) { - indent += indentSize - } - } - - isProcessed = true - if (indentIsAbsolute) { - // don't allow further indenting by parent nodes, this is an absolute position - return indent - } - } - } - - processedLines[srow] = processedLines.getOrDefault(srow, isProcessed) - - node = node.parent - } - - return indent - } - - private fun findDelimiter(content: Content, node: TSNode, - delimiter: String): Pair { - for (i in 0 until node.childCount) { - val child = node.getChild(i) - if (child.type != delimiter) { - continue - } - - val start = node.startPoint - val end = node.endPoint - val line = content.getLine(start.line) - val escapedDelim = delimiter.replace(DELIMITER_REGEX, "\\\\$0") - val trimmedAfterDelim = line.substring((end.column shr 1) + 1) - .replace(Regex("""[\s$escapedDelim]*"""), "") - return child to trimmedAfterDelim.isEmpty() - } - - return null to false - } - - /** - * Get the indents from the query. - * - * @return The indent captures from the query. The returned map has the following structure : - * ``` - * map[indentType][node_id] = capture - * ``` - * where `indentType` is one of the indent types defined in [TreeSitterIndentProvider.Companion]`.IDENT_XXX` - * and `node_id` is same as [TSNode.getNodeId]. - */ - private fun getIndents( - query: TSQuery, - cursor: TSQueryCursor - ): IndentsContainer { - val indents = IndentsContainer() - - var match: TSQueryMatch? = cursor.nextMatch() - while (match != null) { - for (capture in match.captures) { - val captureName = query.getCaptureNameForId(capture.index) - if (!indents.containsKey(captureName)) { - log.warn("Unknown capture name in indents query: {}", captureName) - continue - } - - indents[captureName]!![capture.node.nodeId] = capture to match.metadata - } - match = cursor.nextMatch() - } - - return indents - } - - private inner class IndentsContainer { - - private val data = HashMap>>( - IDENT_TYP_COUNT) - - init { - // pre-fill the indents type so we could report any unknown indent types later - data[IDENT_AUTO] = mutableLongObjectMapOf() - data[IDENT_BEGIN] = mutableLongObjectMapOf() - data[IDENT_END] = mutableLongObjectMapOf() - data[IDENT_DEDENT] = mutableLongObjectMapOf() - data[IDENT_BRANCH] = mutableLongObjectMapOf() - data[IDENT_IGNORE] = mutableLongObjectMapOf() - data[IDENT_ALIGN] = mutableLongObjectMapOf() - data[IDENT_ZERO] = mutableLongObjectMapOf() - } - - fun containsKey(key: String): Boolean { - return data.containsKey(key) - } - - fun get(type: String, node: TSNode) = get(type, node.nodeId) - fun get(type: String, nodeId: Long) = data[type]?.get(nodeId) - - fun hasNode(type: String, node: TSNode) = hasNode(type, node.nodeId) - fun hasNode(type: String, nodeId: Long) = data[type]?.get(nodeId) != null - - fun hasMeta(type: String, node: TSNode, metaKey: String) = hasMeta(type, node.nodeId, metaKey) - fun hasMeta(type: String, nodeId: Long, metaKey: String) = - data[type]?.get(nodeId)?.second?.get(metaKey) != null - - fun getMeta(type: String, node: TSNode) = getMeta(type, node.nodeId) - fun getMeta(type: String, nodeId: Long) = data[type]?.get(nodeId)?.second - - fun getMetaValue(type: String, node: TSNode, metaKey: String) = - getMetaValue(type, node.nodeId, metaKey) - - fun getMetaValue(type: String, nodeId: Long, metaKey: String) = - data[type]?.get(nodeId)?.second?.get(metaKey) - - operator fun get( - key: String): MutableLongObjectMap>? { - return data[key] - } - - operator fun set(key: String, - value: MutableLongObjectMap>) { - data[key] = value - } - } + private fun computeIndentForLine( + content: Content, + line: Int, + column: Int, + default: Int, + rootNode: TSNode, + indents: IndentsContainer, + ): Int { + val isEmptyLine = content.getLine(line).trimmedLength() == 0 + var node: TSNode? + + if (isEmptyLine) { + val prevlnum = content.previousNonBlankLine(line) + if (prevlnum == -1) { + log.error("Cannot compute indents. Unable to get previous non-blank line.") + return default + } else { + log.debug("Previous non-blank line: {}", prevlnum) + } + + var prevline: CharSequence = content.getLine(prevlnum) + val indentBytes = TextUtils.countLeadingSpaceCount(prevline, indentSize) shl 1 + prevline = prevline.trim() + + // The final position can be trailing spaces, which should not affect indentation + node = content.getLastNodeAtLine( + rootNode, + prevlnum, + (indentBytes + prevline.length shl 1) - 2, + ) ?: run { + log.error("Unable to get last node at line: {}", prevlnum) + return default + } + + // TODO(itsaky): Make this an API + // Language defs must be able to specify captures which represent a comment + if (node.type == "comment") { + // The final node we capture of the previous line can be a comment node, which should also be ignored + // Unless the last line is an entire line of comment, ignore the comment range and find the last node again + val firstNode = content.getFirstNodeAtLine(rootNode, prevlnum, indentBytes) + val scol = node.startPoint.column + if (firstNode?.nodeId != node.nodeId) { + // In case the last captured node is a trailing comment node, re-trim the string + prevline = prevline.subSequence(0, (scol shr 1) - (indentBytes shr 1)).trim() + val col = indentBytes + ((prevline.length - 1) shl 1) + + node = content.getLastNodeAtLine(rootNode, prevlnum, col) + } + } + + if (indents[IDENT_END]!![node?.nodeId ?: 0] != null) { + node = content.getFirstNodeAtLine(rootNode, line) + } + } else { + node = content.getFirstNodeAtLine(rootNode, line, column shl 1) + } + + if (node == null || !node.canAccess()) { + log.error( + "Cannot compute indents. Unable to get node at line: {}. node={} node.canAccess={}", + line, + node, + node?.canAccess(), + ) + return default + } + + var indent = 0 + + if (indents[IDENT_ZERO]?.containsKey(node.nodeId) == true) { + // indent.zero: align the node to the start of the line + log.debug("Zero indent for node: {}", node) + return INDENT_ALIGN_ZERO + } + + // map to store whether a given line is already processed + // this is to ensure that we do not accidentally apply multiple indent levels to the same line + val processedLines = mutableIntObjectMapOf() + + while (node != null && node.canAccess()) { + val srow = node.startPoint.line + val erow = node.endPoint.line + + // do 'auto indent' if not marked as '@indent' + if (!indents.hasNode(IDENT_BEGIN, node) && + !indents.hasNode(IDENT_ALIGN, node) && + indents.hasNode(IDENT_AUTO, node) && + srow < line && + line <= erow + ) { + log.debug("Auto indent for node: {}", node) + return INDENT_AUTO + } + + // Do not indent if we are inside an @ignore block. + // If a node spans from L1,C1 to L2,C2, we know that lines where L1 < line <= L2 would + // have their indentations contained by the node. + if (!indents.hasNode(IDENT_BEGIN, node) && + indents.hasNode(IDENT_IGNORE, node) && + srow < line && + line <= erow + ) { + log.debug("Ignore indent for node: {}", node) + return default + } + + var isProcessed = false + + if (!processedLines.containsKey(srow) && + ( + (indents.hasNode(IDENT_BRANCH, node) && srow == line) || + (indents.hasNode(IDENT_DEDENT, node) && srow != line) + ) + ) { + indent -= indentSize + isProcessed = true + } + + // do not indent for nodes that starts-and-ends on same line and starts on target line (lnum) + val shouldProcess = !processedLines.containsKey(srow) + var isInError = false + if (shouldProcess) { + isInError = node.parent?.let { it.canAccess() && it.hasErrors() } == true + } + + if (shouldProcess && + ( + indents.hasNode(IDENT_BEGIN, node) && + ( + srow != erow || isInError || + indents.hasMeta( + IDENT_BEGIN, + node, + "indent.immediate", + ) + ) && + (srow != line || indents.hasMeta(IDENT_BEGIN, node, "indent.start_at_same_line")) + ) + ) { + indent += indentSize + isProcessed = true + } + + if (isInError && !indents.hasNode(IDENT_ALIGN, node)) { + // only when the node is in error, promote the + // first child's aligned indent to the error node + // to work around ((ERROR "X" . (_)) @aligned_indent (#set! "delimiter" "AB")) + // matching for all X, instead set do + // (ERROR "X" @aligned_indent (#set! "delimiter" "AB") . (_)) + // and we will fish it out here. + + for (i in 0 until node.childCount) { + val child = node.getChild(i) + if (indents.hasNode(IDENT_ALIGN, child)) { + indents[IDENT_ALIGN]!![node.nodeId] = indents[IDENT_ALIGN]!![child.nodeId]!! + break + } + } + } + + // do not indent for nodes that starts-and-ends on same line and starts on target line (lnum) + if (shouldProcess && + indents.hasNode(IDENT_ALIGN, node) && + (srow != erow || isInError) && + (srow != line) + ) { + val meta = indents.getMeta(IDENT_ALIGN, node)!! + var oDelimNode: TSNode? + var oIsLastInLine = false + var cDelimNode: TSNode? + var cIsLastInLine = false + var indentIsAbsolute = false + + if (meta.containsKey("indent.open_delimiter")) { + val r = findDelimiter(content, node, meta["indent.open_delimiter"]!!) + oDelimNode = r.first + oIsLastInLine = r.second + } else { + oDelimNode = node + } + + if (meta.containsKey("indent.close_delimiter")) { + val r = findDelimiter(content, node, meta["indent.close_delimiter"]!!) + cDelimNode = r.first + cIsLastInLine = r.second + } else { + cDelimNode = node + } + + if (oDelimNode != null) { + val osrow = oDelimNode.startPoint.row + val oscol = oDelimNode.startPoint.column + var csrow: Int? = null + if (cDelimNode != null) { + csrow = cDelimNode.startPoint.row + } + + if (oIsLastInLine) { + // hanging indent (previous line ended with starting delimiter) + // should be processed like indent + if (shouldProcess) { + indent += indentSize + if (cIsLastInLine) { + // If current line is outside the range of a node marked with `@aligned_indent` + // Then its indent level shouldn't be affected by `@aligned_indent` node + if (csrow != null && csrow < line) { + indent = max(indent - indentSize, 0) + } + } + } + } else { + // aligned indent + if (cIsLastInLine && csrow != null && osrow != csrow && csrow < line) { + // If current line is outside the range of a node marked with `@aligned_indent` + // Then its indent level shouldn't be affected by `@aligned_indent` node + indent = max(indent - indentSize, 0) + } else { + indent = oscol + (meta.getInt("indent.increment") ?: 1) + indentIsAbsolute = true + } + } + + // deal with final line + var avoidLastMatchingNext = false + if (csrow != null && csrow != osrow && csrow == line) { + // delims end on current line, and are not open and closed same line. + // then this last line may need additional indent to avoid clashes + // with the next. `indent.avoid_last_matching_next` controls this behavior, + // for example this is needed for function parameters. + + avoidLastMatchingNext = meta.getBolean("indent.avoid_last_matching_next") + ?: false + } + + if (avoidLastMatchingNext) { + // last line must be indented more in cases where + // it would be same indent as next line (we determine this as one + // width more than the open indent to avoid confusing with any + // hanging indents) + val osrowIndent = TextUtils.countLeadingSpaceCount(content.getLine(osrow), indentSize) + if (indent <= osrowIndent + indentSize) { + indent += indentSize + } + } + + isProcessed = true + if (indentIsAbsolute) { + // don't allow further indenting by parent nodes, this is an absolute position + return indent + } + } + } + + processedLines[srow] = processedLines.getOrDefault(srow, isProcessed) + + node = node.parent + } + + return indent + } + + private fun findDelimiter( + content: Content, + node: TSNode, + delimiter: String, + ): Pair { + for (i in 0 until node.childCount) { + val child = node.getChild(i) + if (child.type != delimiter) { + continue + } + + val start = node.startPoint + val end = node.endPoint + val line = content.getLine(start.line) + val escapedDelim = delimiter.replace(DELIMITER_REGEX, "\\\\$0") + val trimmedAfterDelim = + line + .substring((end.column shr 1) + 1) + .replace(Regex("""[\s$escapedDelim]*"""), "") + return child to trimmedAfterDelim.isEmpty() + } + + return null to false + } + + /** + * Get the indents from the query. + * + * @return The indent captures from the query. The returned map has the following structure : + * ``` + * map[indentType][node_id] = capture + * ``` + * where `indentType` is one of the indent types defined in [TreeSitterIndentProvider.Companion]`.IDENT_XXX` + * and `node_id` is same as [TSNode.getNodeId]. + */ + private fun getIndents( + query: TSQuery, + cursor: TSQueryCursor, + ): IndentsContainer { + val indents = IndentsContainer() + + var match: TSQueryMatch? = cursor.nextMatch() + while (match != null) { + for (capture in match.captures) { + val captureName = query.getCaptureNameForId(capture.index) + if (!indents.containsKey(captureName)) { + log.warn("Unknown capture name in indents query: {}", captureName) + continue + } + + indents[captureName]!![capture.node.nodeId] = capture to match.metadata + } + match = cursor.nextMatch() + } + + return indents + } + + private inner class IndentsContainer { + private val data = + HashMap>>(IDENT_TYP_COUNT) + + init { + // pre-fill the indents type so we could report any unknown indent types later + data[IDENT_AUTO] = mutableLongObjectMapOf() + data[IDENT_BEGIN] = mutableLongObjectMapOf() + data[IDENT_END] = mutableLongObjectMapOf() + data[IDENT_DEDENT] = mutableLongObjectMapOf() + data[IDENT_BRANCH] = mutableLongObjectMapOf() + data[IDENT_IGNORE] = mutableLongObjectMapOf() + data[IDENT_ALIGN] = mutableLongObjectMapOf() + data[IDENT_ZERO] = mutableLongObjectMapOf() + } + + fun containsKey(key: String): Boolean = data.containsKey(key) + + fun get( + type: String, + node: TSNode, + ) = get(type, node.nodeId) + + fun get( + type: String, + nodeId: Long, + ) = data[type]?.get(nodeId) + + fun hasNode( + type: String, + node: TSNode, + ) = hasNode(type, node.nodeId) + + fun hasNode( + type: String, + nodeId: Long, + ) = data[type]?.get(nodeId) != null + + fun hasMeta( + type: String, + node: TSNode, + metaKey: String, + ) = hasMeta(type, node.nodeId, metaKey) + + fun hasMeta( + type: String, + nodeId: Long, + metaKey: String, + ) = data[type]?.get(nodeId)?.second?.get(metaKey) != null + + fun getMeta( + type: String, + node: TSNode, + ) = getMeta(type, node.nodeId) + + fun getMeta( + type: String, + nodeId: Long, + ) = data[type]?.get(nodeId)?.second + + fun getMetaValue( + type: String, + node: TSNode, + metaKey: String, + ) = getMetaValue(type, node.nodeId, metaKey) + + fun getMetaValue( + type: String, + nodeId: Long, + metaKey: String, + ) = data[type]?.get(nodeId)?.second?.get(metaKey) + + operator fun get(key: String): MutableLongObjectMap>? = data[key] + + operator fun set( + key: String, + value: MutableLongObjectMap>, + ) { + data[key] = value + } + } } /** * Alias for [TSPoint.row]. */ private val TSPoint.line: Int - get() = this.row + get() = this.row private fun TSQueryMatch.Metadata.getInt(key: String) = getString(key).toIntOrNull() -private fun TSQueryMatch.Metadata.getBolean(key: String) = getString(key).toBooleanStrictOrNull() \ No newline at end of file + +private fun TSQueryMatch.Metadata.getBolean(key: String) = getString(key).toBooleanStrictOrNull() From 3a9848265d89c467d8165e54b4abf29b9f3d4377 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 16:32:50 -0700 Subject: [PATCH 7/8] ADFA-5401: Guard the indents query too Review caught that the PR claimed to fix "every caller at the mechanism" while TreeSitterIndentProvider was not covered. That claim was wrong: this file never routes through safeExecQueryCursor, and its traversal (getIndents) loops cursor.nextMatch() over languageSpec.indentsQuery with no accessibility check at all - neither before the loop nor inside it. indentsQuery is freed by TreeSitterLanguageSpec.close(), the same teardown chain that frees the highlights query this ticket started from, so the crash shape is identical. Guards it the same way: once before exec(), and per-iteration in the traversal. Converting the whole file to safeExecQueryCursor would be the tidier answer, but it builds an IndentsContainer from every match rather than acting on each one, so the shapes do not line up without restructuring indent computation - which is more than this ticket should carry. Same limit as the rest of this PR: canAccess() is check-then-use, so this narrows the window rather than closing it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../language/treesitter/TreeSitterIndentProvider.kt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt b/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt index f9a981d9eb..bc70b5b2cf 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt @@ -161,6 +161,13 @@ class TreeSitterIndentProvider( optimizeCursorRange(positions, content, cursor) + if (!indentsQuery.canAccess() || !cursor.canAccess()) { + // The spec that owns indentsQuery is closed from the editor's teardown, on another + // thread; running a cursor over a freed query is a native crash (ADFA-5401). + log.info("Cannot compute indents. The indents query is no longer accessible.") + return@use defaultIndents + } + cursor.exec(indentsQuery, tree.rootNode) val indents = getIndents(languageSpec.indentsQuery, cursor) @@ -492,7 +499,11 @@ class TreeSitterIndentProvider( val indents = IndentsContainer() var match: TSQueryMatch? = cursor.nextMatch() - while (match != null) { + + // Re-checked every iteration, not just before the loop: the query is shared and whoever + // frees it does so on another thread, so a free landing mid-traversal would make the next + // nextMatch() dereference a dangling TSQuery (ADFA-5401). + while (match != null && query.canAccess() && cursor.canAccess()) { for (capture in match.captures) { val captureName = query.getCaptureNameForId(capture.index) if (!indents.containsKey(captureName)) { From 059f6912847b82c65af0377199a301f79b123dc1 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 17:23:54 -0700 Subject: [PATCH 8/8] ADFA-5401: Shrink to the guards; revert the teardown changes Three review rounds have now found that every change I made to lifecycle or ordering in this module introduced a race I had not foreseen, while the additive guards held up. So this reverts the mechanism and keeps the guards. Reverted to origin/stage behaviour: - The teardown relocation. Moving document.close() into the analyzer loop's finally created two new races: stop() cancels the job before offering Stop, so a coroutine cancelled before its body was dispatched would never run the finally at all - leaking the native document and the dispatcher thread; and stop() still called requestCancellationAndWaitIfParsing() on the caller thread while the analyzer thread could be closing that same parser. Neither was possible when the close ran on the caller thread. - drainPendingMessages(). Recycling queued TSInputEdits looked like a free win, but TsAnalyzeManager.insert() hands the same instance to LineSpansGenerator.edit(), which applies it asynchronously on a live executor. Recycling from stop() can corrupt a pending apply. messageChannel.clear() is wasteful and safe; this was cheap and unsafe. Also removes STOP_TIMEOUT_MS, which 61fa2ab claimed to remove and did not - the edit silently no-opped and I did not check. What remains, all additive: - query.canAccess() in doSafeExecQueryCursor's per-iteration condition, which is the actual fix: the query was checked once before the loop and never inside it. - A pre-exec indentsQuery.canAccess() check in TreeSitterIndentProvider, falling back to the default indents. The per-iteration guard I had added there is gone: it exited the traversal with a half-built IndentsContainer, which yields silently wrong indentation rather than a fallback. - The Stop sentinel, so a loop parked in a blocking take() leaves promptly, and analyzerContext.close() after document.close() rather than before. - isDestroyed is @Volatile. Known and not fixed here, each getting its own ticket: the shared query has no ownership, so canAccess() remains check-then-use; doSafeExecQueryCursor's top-of-loop exit skips onClosedOrEdited() and the match recycle, which this guard makes reachable more often; and the document can still be closed while the analyzer reads it, which produces a caught IllegalStateException rather than the SIGSEGV this ticket is about. Verified on device (Pixel 6 Pro, Android 17): four open/close cycles with the log panel open, same pid throughout, no SIGSEGV, no "Cannot access native object", no "Analyzer job failed". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../rosemoe/sora/editor/ts/TsAnalyzeWorker.kt | 57 +++---------------- .../treesitter/TreeSitterIndentProvider.kt | 12 ++-- 2 files changed, 12 insertions(+), 57 deletions(-) diff --git a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt index e178106e81..0d2dbdbd08 100644 --- a/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt +++ b/editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt @@ -60,9 +60,6 @@ class TsAnalyzeWorker( ) { companion object { private val log = LoggerFactory.getLogger(TsAnalyzeWorker::class.java) - - /** Upper bound on waiting for the analyzer loop to finish before freeing its document. */ - private const val STOP_TIMEOUT_MS = 500L } var stylesReceiver: StyleReceiver? = null @@ -113,48 +110,17 @@ class TsAnalyzeWorker( document.requestCancellationAndWaitIfParsing() - drainPendingMessages() - - val job = analyzerJob - job?.cancel(CancellationException("Requested to be stopped")) + messageChannel.clear() + analyzerJob?.cancel(CancellationException("Requested to be stopped")) // Cancelling does not wake a thread parked in take(), and closing the dispatcher does not stop // it either - kotlinx reroutes the rejected dispatch to Dispatchers.IO. Hand the loop a message - // so it can see isDestroyed and return; it frees the natives on its way out. + // so it can see isDestroyed and return. messageChannel.offer(Stop) analyzerScope.cancel(CancellationException("Requested to be stopped")) - - if (job == null) { - // start() was never called, so there is no loop to do it. - closeNatives() - } - } - - /** - * Releases the document and the dispatcher. Runs on the analyzer thread once its loop has - * finished, so nothing can still be reading what it frees. - */ - private fun closeNatives() { - try { - document.close() - } finally { - analyzerContext.close() - } - } - - /** - * Empties the queue, recycling each message's pooled natives. [LinkedBlockingQueue.clear] would - * drop them without recycling, and stop() runs on every reset - so that churn would be - * per-interaction, not per-close. - */ - private fun drainPendingMessages() { - while (true) { - when (val pending = messageChannel.poll() ?: return) { - is Mod -> (pending.data.edit as? TreeSitterInputEdit?)?.recycle() - else -> Unit - } - } + document.close() + analyzerContext.close() } fun start() { @@ -163,17 +129,8 @@ class TsAnalyzeWorker( analyzerJob = analyzerScope .launch { - try { - while (!isDestroyed && isActive) { - processNextMessage() - } - } finally { - // Freed here, by the thread that uses them, rather than by whoever calls - // stop(): the loop can be mid doMod()/updateStyles() when stop() runs, and a - // caller-side close would pull the text and tree out from under it. Same idiom - // as LineSpansGenerator, which posts its own tree.close() onto its executor. - // stop() cannot wait for this instead - it is on the setText/reset hot path. - closeNatives() + while (!isDestroyed && isActive) { + processNextMessage() } }.also { job -> job.invokeOnCompletion { error -> diff --git a/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt b/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt index bc70b5b2cf..d9a75fffce 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/language/treesitter/TreeSitterIndentProvider.kt @@ -161,9 +161,11 @@ class TreeSitterIndentProvider( optimizeCursorRange(positions, content, cursor) - if (!indentsQuery.canAccess() || !cursor.canAccess()) { + if (!indentsQuery.canAccess()) { // The spec that owns indentsQuery is closed from the editor's teardown, on another - // thread; running a cursor over a freed query is a native crash (ADFA-5401). + // thread; running a cursor over a freed query is a native crash. Falling back to the + // defaults is the whole answer here - a partially built IndentsContainer would give + // silently wrong indentation instead (ADFA-5401). log.info("Cannot compute indents. The indents query is no longer accessible.") return@use defaultIndents } @@ -499,11 +501,7 @@ class TreeSitterIndentProvider( val indents = IndentsContainer() var match: TSQueryMatch? = cursor.nextMatch() - - // Re-checked every iteration, not just before the loop: the query is shared and whoever - // frees it does so on another thread, so a free landing mid-traversal would make the next - // nextMatch() dereference a dangling TSQuery (ADFA-5401). - while (match != null && query.canAccess() && cursor.canAccess()) { + while (match != null) { for (capture in match.captures) { val captureName = query.getCaptureNameForId(capture.index) if (!indents.containsKey(captureName)) {