Skip to content

Commit 3e583ff

Browse files
committed
fix: race condition in diagnostics
1 parent a306f70 commit 3e583ff

File tree

2 files changed

+127
-10
lines changed

2 files changed

+127
-10
lines changed

vscode-lean4/src/diagnostics.ts

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { DiagnosticCollection, Disposable, EventEmitter } from 'vscode'
2-
import { LeanDiagnostic, LeanPublishDiagnosticsParams, p2cConverter } from './utils/converters'
1+
import { CancellationToken, DiagnosticCollection, Disposable, EventEmitter } from 'vscode'
32
import { DocumentUri } from 'vscode-languageserver-protocol'
3+
import { CoalescingSyncQueue } from './utils/coalescingSyncQueue'
4+
import { LeanDiagnostic, LeanPublishDiagnosticsParams, p2cConverter } from './utils/converters'
45

56
export type DiagnosticChangeKind = 'replace' | 'append'
67

@@ -26,15 +27,39 @@ export class DiagnosticChangeEvent {
2627
}
2728
}
2829

30+
type SyncQueueEntry = {
31+
accumulatedParams: LeanPublishDiagnosticsParams
32+
pendingKind: DiagnosticChangeKind
33+
pendingBatch: LeanDiagnostic[]
34+
}
35+
36+
function combineEntries(existing: SyncQueueEntry, incoming: SyncQueueEntry): SyncQueueEntry {
37+
if (incoming.pendingKind === 'replace') {
38+
return incoming
39+
}
40+
incoming.pendingKind satisfies 'append'
41+
return {
42+
accumulatedParams: incoming.accumulatedParams,
43+
pendingKind: existing.pendingKind,
44+
pendingBatch: [...existing.pendingBatch, ...incoming.pendingBatch],
45+
}
46+
}
47+
2948
export class LeanClientDiagnosticCollection implements Disposable {
3049
readonly vsCodeCollection: DiagnosticCollection
3150
private diags: Map<DocumentUri, LeanPublishDiagnosticsParams> = new Map()
3251

3352
private diagnosticsChangedEmitter = new EventEmitter<DiagnosticChangeEvent>()
3453
onDidChangeDiagnostics = this.diagnosticsChangedEmitter.event
3554

55+
private syncQueue: CoalescingSyncQueue<SyncQueueEntry>
56+
3657
constructor(vsCodeCollection: DiagnosticCollection) {
3758
this.vsCodeCollection = vsCodeCollection
59+
this.syncQueue = new CoalescingSyncQueue(
60+
(uri: string, entry: SyncQueueEntry, token: CancellationToken) => this.syncToCollection(uri, entry, token),
61+
(existing, incoming) => combineEntries(existing, incoming)
62+
)
3863
}
3964

4065
private static determineChangeKind(
@@ -63,20 +88,33 @@ export class LeanClientDiagnosticCollection implements Disposable {
6388
}
6489

6590
const accumulatedParams = { ...params, diagnostics: accumulated }
66-
6791
this.diags.set(accumulatedParams.uri, accumulatedParams)
68-
void this.syncToCollection(accumulatedParams)
69-
this.diagnosticsChangedEmitter.fire(new DiagnosticChangeEvent(kind, params, accumulated))
92+
93+
const entry: SyncQueueEntry = {
94+
accumulatedParams,
95+
pendingKind: kind,
96+
pendingBatch: [...params.diagnostics],
97+
}
98+
99+
this.syncQueue.enqueue(accumulatedParams.uri, entry)
70100
}
71101

72-
private async syncToCollection(p: LeanPublishDiagnosticsParams): Promise<void> {
73-
const nonSilent = p.diagnostics.filter(d => !d.isSilent)
74-
const uri = p2cConverter.asUri(p.uri)
75-
const vsCodeDiags = await p2cConverter.asDiagnostics(nonSilent)
76-
this.vsCodeCollection.set(uri, vsCodeDiags)
102+
private async syncToCollection(_uri: string, entry: SyncQueueEntry, token: CancellationToken): Promise<void> {
103+
const nonSilentDiagnostics = entry.accumulatedParams.diagnostics.filter(d => !d.isSilent)
104+
const vsUri = p2cConverter.asUri(entry.accumulatedParams.uri)
105+
const vsDiags = await p2cConverter.asDiagnostics(nonSilentDiagnostics, token)
106+
if (token.isCancellationRequested) {
107+
return
108+
}
109+
this.vsCodeCollection.set(vsUri, vsDiags)
110+
const collapsedParams = { ...entry.accumulatedParams, diagnostics: entry.pendingBatch }
111+
this.diagnosticsChangedEmitter.fire(
112+
new DiagnosticChangeEvent(entry.pendingKind, collapsedParams, entry.accumulatedParams.diagnostics),
113+
)
77114
}
78115

79116
dispose(): void {
117+
this.syncQueue.dispose()
80118
this.diagnosticsChangedEmitter.dispose()
81119
}
82120
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { CancellationToken, CancellationTokenSource, Disposable } from 'vscode'
2+
3+
/**
4+
* A queue that coalesces pending work by string key, processes one entry at a time,
5+
* and supports cancellation of in-flight work when the same key is re-enqueued.
6+
*
7+
* When a key is enqueued while its worker is already running, the in-flight
8+
* CancellationToken is cancelled. The worker should check the token and bail early.
9+
* If the worker completes without cancellation, the entry is removed from the queue.
10+
* If cancelled or errored, the entry stays so the next enqueue can update it.
11+
*/
12+
export class CoalescingSyncQueue<T> implements Disposable {
13+
private queue: Map<string, T> = new Map()
14+
private state: { kind: 'idle' } | { kind: 'busy'; key: string; tokenSource: CancellationTokenSource } = {
15+
kind: 'idle',
16+
}
17+
private disposed = false
18+
19+
constructor(
20+
private readonly worker: (key: string, value: T, token: CancellationToken) => Promise<void>,
21+
private readonly merge?: (existing: T, incoming: T) => T,
22+
) {}
23+
24+
enqueue(key: string, value: T): void {
25+
if (this.state.kind === 'busy' && this.state.key === key) {
26+
this.state.tokenSource.cancel()
27+
}
28+
29+
const existing = this.queue.get(key)
30+
if (this.merge !== undefined && existing !== undefined) {
31+
this.queue.set(key, this.merge(existing, value))
32+
} else {
33+
this.queue.set(key, value)
34+
}
35+
36+
if (this.state.kind === 'idle') {
37+
this.trigger()
38+
}
39+
}
40+
41+
private trigger(): void {
42+
queueMicrotask(() => void this.work())
43+
}
44+
45+
private async work(): Promise<void> {
46+
if (this.state.kind === 'busy' || this.disposed) {
47+
return
48+
}
49+
50+
const next = this.queue.entries().next()
51+
if (next.done) {
52+
return
53+
}
54+
const [key, value] = next.value
55+
56+
const tokenSource = new CancellationTokenSource()
57+
this.state = { kind: 'busy', key, tokenSource }
58+
try {
59+
await this.worker(key, value, tokenSource.token)
60+
if (!tokenSource.token.isCancellationRequested && !this.disposed) {
61+
this.queue.delete(key)
62+
}
63+
} finally {
64+
tokenSource.dispose()
65+
this.state = { kind: 'idle' }
66+
if (this.queue.size > 0 && !this.disposed) {
67+
this.trigger()
68+
}
69+
}
70+
}
71+
72+
dispose(): void {
73+
this.disposed = true
74+
if (this.state.kind === 'busy') {
75+
this.state.tokenSource.cancel()
76+
}
77+
this.queue.clear()
78+
}
79+
}

0 commit comments

Comments
 (0)