From 4d4efb4f1d8a9a5672f521845844a1ce30537fc6 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 20 Oct 2026 11:22:33 +0200 Subject: [PATCH 1/2] Delete tree cursors --- .../lib-engine/src/actions/ShowParseTree.ts | 7 +- .../getChildNodesForFieldName.ts | 18 ++-- .../src/languages/TreeSitterQuery/isEven.ts | 24 +++--- .../src/util/treeSitterUtils.test.ts | 85 +++++++++++++++++++ .../lib-engine/src/util/treeSitterUtils.ts | 22 +++-- 5 files changed, 129 insertions(+), 27 deletions(-) create mode 100644 packages/lib-engine/src/util/treeSitterUtils.test.ts diff --git a/packages/lib-engine/src/actions/ShowParseTree.ts b/packages/lib-engine/src/actions/ShowParseTree.ts index 1098288a21c..c2975ab3fe1 100644 --- a/packages/lib-engine/src/actions/ShowParseTree.ts +++ b/packages/lib-engine/src/actions/ShowParseTree.ts @@ -43,7 +43,12 @@ function parseTree( const resultPlayground: string[] = []; const resultQuery: string[] = []; - parseCursor(resultPlayground, resultQuery, contentRange, tree.walk(), 0); + const cursor = tree.walk(); + try { + parseCursor(resultPlayground, resultQuery, contentRange, cursor, 0); + } finally { + cursor.delete(); + } return [ `## ${document.filename} [${contentRange}]\n`, diff --git a/packages/lib-engine/src/languages/TreeSitterQuery/getChildNodesForFieldName.ts b/packages/lib-engine/src/languages/TreeSitterQuery/getChildNodesForFieldName.ts index 681f7e163c2..021f1e43396 100644 --- a/packages/lib-engine/src/languages/TreeSitterQuery/getChildNodesForFieldName.ts +++ b/packages/lib-engine/src/languages/TreeSitterQuery/getChildNodesForFieldName.ts @@ -6,14 +6,18 @@ export function getChildNodesForFieldName( ): Node[] { const nodes = []; const treeCursor = node.walk(); - let hasNext = treeCursor.gotoFirstChild(); + try { + let hasNext = treeCursor.gotoFirstChild(); - while (hasNext) { - if (treeCursor.currentFieldName === fieldName) { - nodes.push(treeCursor.currentNode); + while (hasNext) { + if (treeCursor.currentFieldName === fieldName) { + nodes.push(treeCursor.currentNode); + } + hasNext = treeCursor.gotoNextSibling(); } - hasNext = treeCursor.gotoNextSibling(); - } - return nodes; + return nodes; + } finally { + treeCursor.delete(); + } } diff --git a/packages/lib-engine/src/languages/TreeSitterQuery/isEven.ts b/packages/lib-engine/src/languages/TreeSitterQuery/isEven.ts index f641ac0ab2e..0ae417f34dc 100644 --- a/packages/lib-engine/src/languages/TreeSitterQuery/isEven.ts +++ b/packages/lib-engine/src/languages/TreeSitterQuery/isEven.ts @@ -13,18 +13,22 @@ export function isEven(node: Node, fieldName: string): boolean { } const treeCursor = node.parent.walk(); - let hasNext = treeCursor.gotoFirstChild(); - let even = true; + try { + let hasNext = treeCursor.gotoFirstChild(); + let even = true; - while (hasNext) { - if (treeCursor.currentFieldName === fieldName) { - if (treeCursor.currentNode.id === node.id) { - return even; + while (hasNext) { + if (treeCursor.currentFieldName === fieldName) { + if (treeCursor.currentNode.id === node.id) { + return even; + } + even = !even; } - even = !even; + hasNext = treeCursor.gotoNextSibling(); } - hasNext = treeCursor.gotoNextSibling(); - } - throw new Error(`Node not found in parent for field: ${fieldName}`); + throw new Error(`Node not found in parent for field: ${fieldName}`); + } finally { + treeCursor.delete(); + } } diff --git a/packages/lib-engine/src/util/treeSitterUtils.test.ts b/packages/lib-engine/src/util/treeSitterUtils.test.ts new file mode 100644 index 00000000000..4005539c425 --- /dev/null +++ b/packages/lib-engine/src/util/treeSitterUtils.test.ts @@ -0,0 +1,85 @@ +import assert from "node:assert/strict"; +import { mock } from "node:test"; +import type { Tree, TreeCursor } from "web-tree-sitter"; +import { Node } from "web-tree-sitter"; +import { FakeIDE, InMemoryTextEditor } from "@cursorless/lib-common"; +import { getChildNodesForFieldName } from "../languages/TreeSitterQuery/getChildNodesForFieldName"; +import { isEven } from "../languages/TreeSitterQuery/isEven"; +import { TestTreeSitter } from "../testUtil/TestTreeSitter"; +import { getChildNodesForFieldName as getChildren } from "./treeSitterUtils"; + +suite("Tree-sitter cursor lifetime", () => { + const treeSitter = new TestTreeSitter(); + let tree: Tree; + const deletions: { + cursor: TreeCursor; + deletion: ReturnType; + }[] = []; + + suiteSetup(async () => { + await treeSitter.loadLanguage("clojure"); + }); + + setup(() => { + const { document } = new InMemoryTextEditor({ + ide: new FakeIDE(), + content: "{:a 1 :b 2}", + languageId: "clojure", + }); + tree = treeSitter.getTree(document); + const walk = Node.prototype.walk; + mock.method(Node.prototype, "walk", function (this: Node) { + const cursor = walk.call(this); + deletions.push({ cursor, deletion: mock.method(cursor, "delete") }); + return cursor; + }); + }); + + teardown(() => { + try { + assert.ok(deletions.length > 0, "Expected traversal to create a cursor"); + for (const { deletion } of deletions) { + assert.equal(deletion.mock.callCount(), 1); + } + } finally { + // Also clean up cursors if a regression leaves them unfreed. Their WASM + // finalizers can crash the process, obscuring the assertion failure. + for (const { cursor, deletion } of deletions) { + if (deletion.mock.callCount() === 0) { + cursor.delete(); + } + } + deletions.length = 0; + mock.restoreAll(); + tree.delete(); + } + }); + + test("releases the cursor on an early return from isEven", () => { + const node = tree.rootNode.descendantsOfType("kwd_lit")[0]; + assert.equal(isEven(node, "value"), true); + }); + + test("releases the cursor when isEven throws for a missing field", () => { + const node = tree.rootNode.descendantsOfType("kwd_lit")[0]; + assert.throws(() => isEven(node, "missing"), /Node not found in parent/u); + }); + + for (const [name, getNodes] of [ + ["query", getChildNodesForFieldName], + ["utility", getChildren], + ] as const) { + test(`${name} traversal releases the cursor after collecting children`, () => { + const node = tree.rootNode.descendantsOfType("map_lit")[0]; + assert.deepEqual( + getNodes(node, "value").map((child) => child.text), + [":a", "1", ":b", "2"], + ); + }); + + test(`${name} traversal releases the cursor for a leaf`, () => { + const node = tree.rootNode.descendantsOfType("num_lit")[0]; + assert.deepEqual(getNodes(node, "value"), []); + }); + } +}); diff --git a/packages/lib-engine/src/util/treeSitterUtils.ts b/packages/lib-engine/src/util/treeSitterUtils.ts index 39f7e4ca1d1..284f46047da 100644 --- a/packages/lib-engine/src/util/treeSitterUtils.ts +++ b/packages/lib-engine/src/util/treeSitterUtils.ts @@ -19,21 +19,25 @@ export function getChildNodesForFieldName( fieldName: string, ): Node[] { const treeCursor = node.walk(); - treeCursor.gotoFirstChild(); + try { + treeCursor.gotoFirstChild(); - const ret = []; + const ret = []; - let hasNext = true; + let hasNext = true; - while (hasNext) { - if (treeCursor.currentFieldName === fieldName) { - ret.push(treeCursor.currentNode); + while (hasNext) { + if (treeCursor.currentFieldName === fieldName) { + ret.push(treeCursor.currentNode); + } + + hasNext = treeCursor.gotoNextSibling(); } - hasNext = treeCursor.gotoNextSibling(); + return ret; + } finally { + treeCursor.delete(); } - - return ret; } /** From 7089e4f5e963a8d5644587ece4bde9874c63b072 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 20 Oct 2026 12:26:20 +0200 Subject: [PATCH 2/2] Log test errors --- .../src/suite/recorded.vscode.test.ts | 55 ++++++++++++++++--- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/test-vscode-e2e/src/suite/recorded.vscode.test.ts b/packages/test-vscode-e2e/src/suite/recorded.vscode.test.ts index 67a42847df8..b417bc02862 100644 --- a/packages/test-vscode-e2e/src/suite/recorded.vscode.test.ts +++ b/packages/test-vscode-e2e/src/suite/recorded.vscode.test.ts @@ -11,7 +11,9 @@ import { } from "@cursorless/lib-node-common"; import { getReusableEditor, + getParseTreeApi, getTestHelpers, + resetReusableEditor, runCursorlessCommand, } from "@cursorless/lib-vscode-common"; import { endToEndTestSetup, sleepWithBackoff } from "../endToEndTestSetup"; @@ -36,19 +38,56 @@ suite("recorded test cases", function () { this.ctx.skip(); } - await runRecordedTest({ - path, - spyIde: getSpy(), - openNewTestEditor, - sleepWithBackoff, - testHelpers: await getTestHelpers(), - runCursorlessCommand, - }); + try { + await runRecordedTest({ + path, + spyIde: getSpy(), + openNewTestEditor, + sleepWithBackoff, + testHelpers: await getTestHelpers(), + runCursorlessCommand, + }); + } catch (error) { + // A retry must not inherit the failed attempt's selections or + // incrementally updated parse tree. + resetReusableEditor(); + await logFailureContext(name); + throw error; + } }), ); } }); +async function logFailureContext(name: string): Promise { + const editor = vscode.window.activeTextEditor; + if (editor == null) { + console.error(`Recorded test failed: ${name}; no active editor`); + return; + } + + const { document, selections } = editor; + console.error(`Recorded test failed: ${name}`, { + uri: document.uri.toString(), + languageId: document.languageId, + version: document.version, + content: document.getText(), + selections: selections.map(({ anchor, active }) => ({ anchor, active })), + }); + + try { + const parseTree = await getParseTreeApi(); + console.error( + "Parse tree:", + parseTree.getTreeForUri(document.uri).rootNode.toString(), + ); + } catch (error) { + // Diagnostics should preserve the original test failure, including for + // languages without Tree-sitter support. + console.error("Could not read parse tree:", error); + } +} + async function openNewTestEditor( content: string, languageId: string,