From 68ed840eaff2e42fbd6f2ee72be35b58d973aca3 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 18:18:22 -0700 Subject: [PATCH 1/2] ADFA-5414: Clean up on both exits from the query cursor loop doSafeExecQueryCursor had two ways out and only one of them cleaned up. The check after action(match) called onClosedOrEdited() and recycled the match before breaking; the loop condition itself could also become false, and that path exited straight out - the caller's compensating action never ran and the match it was holding never went back to the pool. That matters because onClosedOrEdited is how a caller discards a partial result. updateCodeBlocks passes { blocks.clear() }, so an exit through the loop condition committed a half-built list and the editor drew a truncated set of fold and indent guides instead of none. TsScopedVariables passes { captures.clear() } and would commit a torn scope tree. The loop is now driven by `match != null` - normal exhaustion, which must not trigger onClosedOrEdited - with the condition re-checked at the top as well as after the action. Both failure exits run onClosedOrEdited() and recycle; the whileTrue exit recycles without it, since the caller stopping on purpose is not an error. Pre-existing, but ADFA-5401 widened it by adding query.canAccess() to matchCondition, so a freed query now reaches this exit as well as an edited node. Also corrects the debug diagnostic beside it, which printed node.hasErrors() under a "node.hasChanges" label - matchCondition tests hasChanges, so the log pointed at the wrong thing exactly when someone would be reading it. Not addressed here: the early `return result` path still skips the node.recycle() below the loop. That is a separate leak and a separate decision about whether a returned result may reference the node. Verified on device (Pixel 6 Pro, Android 17): syntax highlighting renders and updates normally - this loop is what feeds it - with no "AnalyzeWorker crashed", no "Cannot access native object" and no SIGSEGV. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../androidide/treesitter/api/tsUtils.kt | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 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 e8f249447c..ceaf9faa52 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 @@ -152,22 +152,33 @@ if (!node.canAccess() || node.hasChanges()) { } exec(query, node) + +// Both ways out of this loop have to clean up the same way. The condition is re-checked at the +// top as well as after the action, because the query, cursor and node are shared and can be +// closed or edited by another thread at any point; whichever check trips, the caller's +// onClosedOrEdited must run so it can discard whatever it had half-built, and the match it was +// holding has to go back to the pool (ADFA-5414). var match = nextMatch() -while (matchCondition(match) && whileTrue(match)) { +while (match != null) { + if (!matchCondition(match)) { + logCannotProceed(query, node, debugName, debugLogging) + onClosedOrEdited() + (match as? TreeSitterQueryMatch?)?.recycle() + break + } + + if (!whileTrue(match)) { + // The caller asked to stop; nothing is wrong, so no onClosedOrEdited. + (match as? TreeSitterQueryMatch?)?.recycle() + break + } 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()}" - ) - } + logCannotProceed(query, node, debugName, debugLogging) onClosedOrEdited() + (match as? TreeSitterQueryMatch?)?.recycle() break } @@ -188,3 +199,26 @@ if (recycleNodeAfterUse && node is TreeSitterNode && !node.isRecycled) { return null } + +/** + * Diagnostic for the two places [doSafeExecQueryCursor] gives up on a traversal. + */ +@PublishedApi +internal fun TSQueryCursor.logCannotProceed( + query: TSQuery, + node: TSNode, + debugName: String, + debugLogging: Boolean, +) { + if (!debugLogging) { + return + } + + log.debug( + "$debugName: Cannot proceed with query operation.", + "cursor.canAccess=${canAccess()}", + "query.canAccess=${query.canAccess()}", + "node.canAccess=${node.canAccess()}", + "node.hasChanges=${node.canAccess() && node.hasChanges()}", + ) +} From aa973a84d58b13b0d2e2fd40c28cea1aa14f5ea1 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 18:18:22 -0700 Subject: [PATCH 2/2] ADFA-5416: Check blocksQuery.canAccess() before reading patternCount updateCodeBlocks() guarded on blocksQuery.patternCount == 0 || !blocksQuery.canAccess() || ... but TSQuery.getPatternCount() calls checkAccess() internally, so on a closed query the first operand throws IllegalStateException before the guard beside it can return. The exception left updateStyles() and was swallowed by processNextMessage()'s catch as "AnalyzeWorker crashed", dropping the whole style update rather than skipping just the code-blocks step the guard existed to skip. Swapped, matching TsBracketPairs, which already had the operands this way round. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 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 0d2dbdbd08..302863b883 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 @@ -328,8 +328,11 @@ class TsAnalyzeWorker( } private fun updateCodeBlocks() { - if (languageSpec.blocksQuery.patternCount == 0 || - !languageSpec.blocksQuery.canAccess() || + // canAccess() first: getPatternCount() calls checkAccess() internally, so on a closed query + // reading patternCount throws before the guard beside it can return (ADFA-5416). The sibling + // site in TsBracketPairs already has the operands this way round. + if (!languageSpec.blocksQuery.canAccess() || + languageSpec.blocksQuery.patternCount == 0 || tree?.canAccess() != true ) { return