From 6c67db00e89026e3c8c77651ceedb2a082d33ac5 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Mon, 24 Jun 2024 18:58:41 +0200 Subject: [PATCH 1/2] Fix data race in problem view tree --- .../problem/problem-composite-tree-node.ts | 7 +++++- .../src/browser/problem/problem-tree-model.ts | 22 ++++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/markers/src/browser/problem/problem-composite-tree-node.ts b/packages/markers/src/browser/problem/problem-composite-tree-node.ts index a5b20be14516b..5cdd426d05bde 100644 --- a/packages/markers/src/browser/problem/problem-composite-tree-node.ts +++ b/packages/markers/src/browser/problem/problem-composite-tree-node.ts @@ -23,6 +23,11 @@ import { ProblemUtils } from './problem-utils'; export namespace ProblemCompositeTreeNode { + export interface Child { + node: MarkerInfoNode; + markers: Marker[]; + } + export function setSeverity(parent: MarkerInfoNode, markers: Marker[]): void { let maxSeverity: DiagnosticSeverity | undefined; markers.forEach(marker => { @@ -33,7 +38,7 @@ export namespace ProblemCompositeTreeNode { parent.severity = maxSeverity; }; - export function addChildren(parent: CompositeTreeNode, insertChildren: { node: MarkerInfoNode, markers: Marker[] }[]): void { + export function addChildren(parent: CompositeTreeNode, insertChildren: ProblemCompositeTreeNode.Child[]): void { for (const { node, markers } of insertChildren) { ProblemCompositeTreeNode.setSeverity(node, markers); } diff --git a/packages/markers/src/browser/problem/problem-tree-model.ts b/packages/markers/src/browser/problem/problem-tree-model.ts index 152feb7a33408..c7d362d7f9e71 100644 --- a/packages/markers/src/browser/problem/problem-tree-model.ts +++ b/packages/markers/src/browser/problem/problem-tree-model.ts @@ -28,7 +28,8 @@ import debounce = require('@theia/core/shared/lodash.debounce'); @injectable() export class ProblemTree extends MarkerTree { - protected markers: { node: MarkerInfoNode, markers: Marker[] }[] = []; + + protected queuedMarkers = new Map(); constructor( @inject(ProblemManager) markerManager: ProblemManager, @@ -37,6 +38,11 @@ export class ProblemTree extends MarkerTree { super(markerManager, markerOptions); } + override get root(): MarkerRootNode | undefined { + const superRoot = super.root; + return MarkerRootNode.is(superRoot) ? superRoot : undefined; + } + protected override getMarkerNodes(parent: MarkerInfoNode, markers: Marker[]): MarkerNode[] { const nodes = super.getMarkerNodes(parent, markers); return nodes.sort((a, b) => this.sortMarkers(a, b)); @@ -79,20 +85,26 @@ export class ProblemTree extends MarkerTree { } protected override insertNodeWithMarkers(node: MarkerInfoNode, markers: Marker[]): void { - this.markers.push({ node, markers }); + // Add the element to the queue. + // In case a diagnostics collection for the same file already exists, it will be replaced. + this.queuedMarkers.set(node.id, { node, markers }); this.doInsertNodesWithMarkers(); } protected doInsertNodesWithMarkers = debounce(() => { - ProblemCompositeTreeNode.addChildren(this.root as MarkerRootNode, this.markers); + if (!this.root) { + return; + } + const queuedItems = Array.from(this.queuedMarkers.values()); + ProblemCompositeTreeNode.addChildren(this.root, queuedItems); - for (const { node, markers } of this.markers) { + for (const { node, markers } of queuedItems) { const children = this.getMarkerNodes(node, markers); node.numberOfMarkers = markers.length; this.setChildren(node, children); } - this.markers.length = 0; + this.queuedMarkers.clear(); }, 50); } From be55d2a1be811b80e86e2d8bd126b614615fd467 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Tue, 25 Jun 2024 09:26:49 +0000 Subject: [PATCH 2/2] Fix test error --- .../markers/src/browser/problem/problem-tree-model.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/markers/src/browser/problem/problem-tree-model.ts b/packages/markers/src/browser/problem/problem-tree-model.ts index c7d362d7f9e71..d4eab7184bd83 100644 --- a/packages/markers/src/browser/problem/problem-tree-model.ts +++ b/packages/markers/src/browser/problem/problem-tree-model.ts @@ -38,11 +38,6 @@ export class ProblemTree extends MarkerTree { super(markerManager, markerOptions); } - override get root(): MarkerRootNode | undefined { - const superRoot = super.root; - return MarkerRootNode.is(superRoot) ? superRoot : undefined; - } - protected override getMarkerNodes(parent: MarkerInfoNode, markers: Marker[]): MarkerNode[] { const nodes = super.getMarkerNodes(parent, markers); return nodes.sort((a, b) => this.sortMarkers(a, b)); @@ -92,11 +87,13 @@ export class ProblemTree extends MarkerTree { } protected doInsertNodesWithMarkers = debounce(() => { - if (!this.root) { + const root = this.root; + // Sanity check; This should always be of type `MarkerRootNode` + if (!MarkerRootNode.is(root)) { return; } const queuedItems = Array.from(this.queuedMarkers.values()); - ProblemCompositeTreeNode.addChildren(this.root, queuedItems); + ProblemCompositeTreeNode.addChildren(root, queuedItems); for (const { node, markers } of queuedItems) { const children = this.getMarkerNodes(node, markers);