Skip to content

Commit 3760c71

Browse files
16bit-ykikoclaude
andcommitted
fix: data race in stateful worker and whole-document didChange parsing
Stateful worker race fix: - Move params-to-doc copy after strand lock so concurrent Compile requests can't overwrite fields while et::queue reads them - DocumentUpdate only marks dirty; no longer writes doc.text from event loop while thread pool may be reading it (data race) - Remove unused text field from DocumentUpdateParams didChange whole-document fix: - Eventide's variant deserialization always picks index 0 (TextDocumentContentChangePartial) even for whole-document changes that have no range field. The default range {0,0}-{0,0} caused replace(0, 0, text) to prepend new text instead of replacing. - Detect default range and treat as full replacement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c59e150 commit 3760c71

File tree

3 files changed

+333
-11
lines changed

3 files changed

+333
-11
lines changed

src/server/master_server.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,7 +1373,7 @@ void MasterServer::register_handlers() {
13731373
auto& doc = it->second;
13741374
doc.version = params.text_document.version;
13751375

1376-
// Apply incremental changes
1376+
// Apply content changes.
13771377
for(auto& change: params.content_changes) {
13781378
std::visit(
13791379
[&](auto& c) {
@@ -1382,14 +1382,22 @@ void MasterServer::register_handlers() {
13821382
protocol::TextDocumentContentChangeWholeDocument>) {
13831383
doc.text = c.text;
13841384
} else {
1385-
// Incremental change: replace range
13861385
auto& range = c.range;
1387-
1388-
lsp::PositionMapper mapper(doc.text, lsp::PositionEncoding::UTF16);
1389-
auto start = mapper.to_offset(range.start);
1390-
auto end = mapper.to_offset(range.end);
1391-
if(start && end && *start <= *end) {
1392-
doc.text.replace(*start, *end - *start, c.text);
1386+
// Workaround: eventide variant deserialization always
1387+
// picks TextDocumentContentChangePartial (index 0).
1388+
// When the client sends a whole-document change (no range
1389+
// in JSON), the range defaults to {0,0}-{0,0}. Detect
1390+
// this and treat as a full replacement.
1391+
if(range.start.line == 0 && range.start.character == 0 &&
1392+
range.end.line == 0 && range.end.character == 0) {
1393+
doc.text = c.text;
1394+
} else {
1395+
lsp::PositionMapper mapper(doc.text, lsp::PositionEncoding::UTF16);
1396+
auto start = mapper.to_offset(range.start);
1397+
auto end = mapper.to_offset(range.end);
1398+
if(start && end && *start <= *end) {
1399+
doc.text.replace(*start, *end - *start, c.text);
1400+
}
13931401
}
13941402
}
13951403
},

src/server/stateful_worker.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,6 @@ void StatefulWorker::register_handlers() {
172172
}
173173

174174
auto compile_result = co_await et::queue([&]() -> worker::CompileResult {
175-
LOG_DEBUG("Compiling: path={}, {} args", params.path, doc.arguments.size());
176-
177175
ScopedTimer timer;
178176

179177
CompilationParams cp;

tests/integration/test_staleness.py

Lines changed: 317 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,14 @@
1212

1313
import pytest
1414
from lsprotocol.types import (
15+
DidChangeTextDocumentParams,
16+
DidCloseTextDocumentParams,
1517
DidSaveTextDocumentParams,
1618
HoverParams,
1719
Position,
20+
TextDocumentContentChangeWholeDocument,
1821
TextDocumentIdentifier,
22+
VersionedTextDocumentIdentifier,
1923
)
2024

2125

@@ -41,6 +45,11 @@ def _doc(uri: str) -> TextDocumentIdentifier:
4145
return TextDocumentIdentifier(uri=uri)
4246

4347

48+
# =========================================================================
49+
# Staleness detection tests
50+
# =========================================================================
51+
52+
4453
async def test_header_change_invalidates_ast(client, tmp_path):
4554
"""Modifying a header on disk should cause recompilation on next hover,
4655
even though didSave was never called (mtime-based detection)."""
@@ -58,7 +67,6 @@ async def test_header_change_invalidates_ast(client, tmp_path):
5867
assert len(diags) == 0, f"Expected clean compile, got: {diags}"
5968

6069
# Modify header on disk — introduce an error.
61-
# Sleep briefly to ensure mtime changes (filesystem granularity).
6270
# Ensure mtime advances past filesystem granularity (1s on some FSes).
6371
await asyncio.sleep(1.1)
6472
(tmp_path / "header.h").write_text(
@@ -131,6 +139,314 @@ async def test_no_change_skips_recompile(client, tmp_path):
131139
assert hover is not None
132140

133141

142+
async def test_touch_without_content_change_skips_recompile(client, tmp_path):
143+
"""Layer 2: touching a header (mtime changes) without modifying content
144+
should NOT trigger recompilation — the hash check catches this."""
145+
(tmp_path / "header.h").write_text("inline int value() { return 1; }\n")
146+
(tmp_path / "main.cpp").write_text(
147+
'#include "header.h"\nint main() { return value(); }\n'
148+
)
149+
_write_cdb(tmp_path, ["main.cpp"])
150+
await client.initialize(tmp_path)
151+
152+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
153+
diags = client.diagnostics.get(uri, [])
154+
assert len(diags) == 0
155+
156+
# Touch the header — mtime changes but content stays the same.
157+
await asyncio.sleep(1.1)
158+
original_content = (tmp_path / "header.h").read_text()
159+
(tmp_path / "header.h").write_text(original_content)
160+
161+
# Hover triggers ensure_compiled which runs deps_changed.
162+
# Layer 2 hash confirms nothing actually changed → cached AST reused.
163+
# Hover on "main" (line 1, col 4) which should be hoverable.
164+
hover = await client.text_document_hover_async(
165+
HoverParams(text_document=_doc(uri), position=Position(line=1, character=4))
166+
)
167+
assert hover is not None
168+
169+
# No new diagnostics should appear — the file is still clean.
170+
diags = client.diagnostics.get(uri, [])
171+
assert len(diags) == 0
172+
173+
174+
async def test_header_replaced_with_different_content(client, tmp_path):
175+
"""Replacing a header file with different content should be detected
176+
and trigger recompilation reflecting the new content."""
177+
(tmp_path / "header.h").write_text("inline int value() { return 1; }\n")
178+
(tmp_path / "main.cpp").write_text(
179+
'#include "header.h"\nint main() { return value(); }\n'
180+
)
181+
_write_cdb(tmp_path, ["main.cpp"])
182+
await client.initialize(tmp_path)
183+
184+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
185+
diags = client.diagnostics.get(uri, [])
186+
assert len(diags) == 0
187+
188+
# Replace header — delete and recreate with a breaking change.
189+
await asyncio.sleep(1.1)
190+
(tmp_path / "header.h").unlink()
191+
(tmp_path / "header.h").write_text(
192+
"inline int renamed_value() { return 1; }\n"
193+
)
194+
195+
# main.cpp still calls value() which no longer exists → error.
196+
event = client.wait_for_diagnostics(uri)
197+
await client.text_document_hover_async(
198+
HoverParams(text_document=_doc(uri), position=Position(line=0, character=0))
199+
)
200+
await asyncio.wait_for(event.wait(), timeout=60.0)
201+
202+
diags = client.diagnostics.get(uri, [])
203+
assert len(diags) > 0, "Expected diagnostics after header replacement"
204+
205+
206+
async def test_fix_error_clears_diagnostics(client, tmp_path):
207+
"""After introducing and fixing an error in a header, diagnostics
208+
should clear on the next recompilation cycle."""
209+
(tmp_path / "header.h").write_text("inline int value() { return }\n") # broken
210+
(tmp_path / "main.cpp").write_text(
211+
'#include "header.h"\nint main() { return value(); }\n'
212+
)
213+
_write_cdb(tmp_path, ["main.cpp"])
214+
await client.initialize(tmp_path)
215+
216+
# First compile — should produce diagnostics.
217+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
218+
diags = client.diagnostics.get(uri, [])
219+
assert len(diags) > 0, "Expected diagnostics from broken header"
220+
221+
# Fix the header.
222+
await asyncio.sleep(1.1)
223+
(tmp_path / "header.h").write_text("inline int value() { return 1; }\n")
224+
225+
# Hover triggers recompilation — diagnostics should clear.
226+
event = client.wait_for_diagnostics(uri)
227+
await client.text_document_hover_async(
228+
HoverParams(text_document=_doc(uri), position=Position(line=0, character=0))
229+
)
230+
await asyncio.wait_for(event.wait(), timeout=60.0)
231+
232+
diags = client.diagnostics.get(uri, [])
233+
assert len(diags) == 0, f"Expected clean compile after fix, got: {diags}"
234+
235+
236+
async def test_multiple_files_share_header(client, tmp_path):
237+
"""When a shared header changes, all open files that depend on it
238+
should detect the staleness independently."""
239+
(tmp_path / "shared.h").write_text("inline int shared() { return 1; }\n")
240+
(tmp_path / "a.cpp").write_text(
241+
'#include "shared.h"\nint fa() { return shared(); }\n'
242+
)
243+
(tmp_path / "b.cpp").write_text(
244+
'#include "shared.h"\nint fb() { return shared(); }\n'
245+
)
246+
_write_cdb(tmp_path, ["a.cpp", "b.cpp"])
247+
await client.initialize(tmp_path)
248+
249+
uri_a, _ = await client.open_and_wait(tmp_path / "a.cpp")
250+
uri_b, _ = await client.open_and_wait(tmp_path / "b.cpp")
251+
assert len(client.diagnostics.get(uri_a, [])) == 0
252+
assert len(client.diagnostics.get(uri_b, [])) == 0
253+
254+
# Break the shared header.
255+
await asyncio.sleep(1.1)
256+
(tmp_path / "shared.h").write_text("inline int shared() { return }\n")
257+
258+
# Both files should get diagnostics after hover.
259+
event_a = client.wait_for_diagnostics(uri_a)
260+
await client.text_document_hover_async(
261+
HoverParams(text_document=_doc(uri_a), position=Position(line=0, character=0))
262+
)
263+
await asyncio.wait_for(event_a.wait(), timeout=60.0)
264+
assert len(client.diagnostics.get(uri_a, [])) > 0, "File A should have diagnostics"
265+
266+
event_b = client.wait_for_diagnostics(uri_b)
267+
await client.text_document_hover_async(
268+
HoverParams(text_document=_doc(uri_b), position=Position(line=0, character=0))
269+
)
270+
await asyncio.wait_for(event_b.wait(), timeout=60.0)
271+
assert len(client.diagnostics.get(uri_b, [])) > 0, "File B should have diagnostics"
272+
273+
274+
async def test_transitive_header_change(client, tmp_path):
275+
"""A change to a transitively included header should be detected."""
276+
(tmp_path / "base.h").write_text("inline int base() { return 1; }\n")
277+
(tmp_path / "mid.h").write_text('#include "base.h"\n')
278+
(tmp_path / "main.cpp").write_text(
279+
'#include "mid.h"\nint main() { return base(); }\n'
280+
)
281+
_write_cdb(tmp_path, ["main.cpp"])
282+
await client.initialize(tmp_path)
283+
284+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
285+
assert len(client.diagnostics.get(uri, [])) == 0
286+
287+
# Modify the transitive dep (base.h).
288+
await asyncio.sleep(1.1)
289+
(tmp_path / "base.h").write_text("inline int base() { return }\n") # broken
290+
291+
event = client.wait_for_diagnostics(uri)
292+
await client.text_document_hover_async(
293+
HoverParams(text_document=_doc(uri), position=Position(line=0, character=0))
294+
)
295+
await asyncio.wait_for(event.wait(), timeout=60.0)
296+
297+
diags = client.diagnostics.get(uri, [])
298+
assert len(diags) > 0, "Expected diagnostics from transitive header change"
299+
300+
301+
# =========================================================================
302+
# didChange / didOpen / didSave / didClose lifecycle tests
303+
# =========================================================================
304+
305+
306+
async def test_didchange_body_edit_recompiles(client, tmp_path):
307+
"""Editing the body (not preamble) via didChange should trigger
308+
recompilation and update diagnostics."""
309+
(tmp_path / "main.cpp").write_text("int main() { return 0; }\n")
310+
_write_cdb(tmp_path, ["main.cpp"])
311+
await client.initialize(tmp_path)
312+
313+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
314+
assert len(client.diagnostics.get(uri, [])) == 0
315+
316+
# Introduce a body error via didChange.
317+
event = client.wait_for_diagnostics(uri)
318+
client.text_document_did_change(
319+
DidChangeTextDocumentParams(
320+
text_document=VersionedTextDocumentIdentifier(uri=uri, version=1),
321+
content_changes=[
322+
TextDocumentContentChangeWholeDocument(
323+
text="int main() { return }\n" # missing expression
324+
)
325+
],
326+
)
327+
)
328+
await client.text_document_hover_async(
329+
HoverParams(text_document=_doc(uri), position=Position(line=0, character=4))
330+
)
331+
await asyncio.wait_for(event.wait(), timeout=30.0)
332+
333+
diags = client.diagnostics.get(uri, [])
334+
assert len(diags) > 0, "Expected diagnostics after body error"
335+
336+
337+
async def test_didchange_preamble_edit_recompiles(client, tmp_path):
338+
"""Changing a preamble #include via didChange should trigger PCH rebuild
339+
and recompilation reflecting the new header's declarations."""
340+
(tmp_path / "a.h").write_text("#pragma once\ninline int from_a() { return 1; }\n")
341+
(tmp_path / "b.h").write_text("#pragma once\ninline int from_b() { return 2; }\n")
342+
(tmp_path / "main.cpp").write_text(
343+
'#include "a.h"\nint main() { return from_a(); }\n'
344+
)
345+
_write_cdb(tmp_path, ["main.cpp"])
346+
await client.initialize(tmp_path)
347+
348+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
349+
assert len(client.diagnostics.get(uri, [])) == 0
350+
351+
# Switch from a.h to b.h and call from_b() instead.
352+
event = client.wait_for_diagnostics(uri)
353+
client.text_document_did_change(
354+
DidChangeTextDocumentParams(
355+
text_document=VersionedTextDocumentIdentifier(uri=uri, version=1),
356+
content_changes=[
357+
TextDocumentContentChangeWholeDocument(
358+
text='#include "b.h"\nint main() { return from_b(); }\n'
359+
)
360+
],
361+
)
362+
)
363+
await client.text_document_hover_async(
364+
HoverParams(text_document=_doc(uri), position=Position(line=1, character=4))
365+
)
366+
await asyncio.wait_for(event.wait(), timeout=30.0)
367+
368+
# Should compile cleanly — from_b() is available via b.h.
369+
diags = client.diagnostics.get(uri, [])
370+
assert len(diags) == 0, f"Expected clean compile after preamble switch, got: {diags}"
371+
372+
373+
async def test_didclose_then_reopen(client, tmp_path):
374+
"""Closing and reopening a file should work correctly — the server
375+
should not retain stale state from the previous session."""
376+
(tmp_path / "main.cpp").write_text("int main() { return 0; }\n")
377+
_write_cdb(tmp_path, ["main.cpp"])
378+
await client.initialize(tmp_path)
379+
380+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
381+
assert len(client.diagnostics.get(uri, [])) == 0
382+
383+
# Close the file.
384+
client.text_document_did_close(
385+
DidCloseTextDocumentParams(text_document=_doc(uri))
386+
)
387+
388+
# Modify on disk while closed.
389+
await asyncio.sleep(1.1)
390+
(tmp_path / "main.cpp").write_text("int main() { return }\n") # broken
391+
392+
# Reopen — should compile the new (broken) content from disk.
393+
uri2, _ = await client.open_and_wait(tmp_path / "main.cpp")
394+
diags = client.diagnostics.get(uri2, [])
395+
assert len(diags) > 0, "Expected diagnostics after reopen with broken content"
396+
397+
398+
async def test_didclose_clears_hover(client, tmp_path):
399+
"""After didClose, hover on the closed file should return None."""
400+
(tmp_path / "main.cpp").write_text("int main() { return 0; }\n")
401+
_write_cdb(tmp_path, ["main.cpp"])
402+
await client.initialize(tmp_path)
403+
404+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
405+
406+
client.text_document_did_close(
407+
DidCloseTextDocumentParams(text_document=_doc(uri))
408+
)
409+
410+
hover = await client.text_document_hover_async(
411+
HoverParams(text_document=_doc(uri), position=Position(line=0, character=4))
412+
)
413+
assert hover is None, "Hover on closed file should return None"
414+
415+
416+
async def test_didsave_triggers_recompile_for_dependents(client, tmp_path):
417+
"""didSave on a header file should mark dependent documents dirty."""
418+
(tmp_path / "header.h").write_text("inline int value() { return 1; }\n")
419+
(tmp_path / "main.cpp").write_text(
420+
'#include "header.h"\nint main() { return value(); }\n'
421+
)
422+
_write_cdb(tmp_path, ["main.cpp"])
423+
await client.initialize(tmp_path)
424+
425+
uri, _ = await client.open_and_wait(tmp_path / "main.cpp")
426+
assert len(client.diagnostics.get(uri, [])) == 0
427+
428+
# Modify header on disk and send didSave.
429+
await asyncio.sleep(1.1)
430+
(tmp_path / "header.h").write_text("inline int value() { return }\n") # broken
431+
client.text_document_did_save(
432+
DidSaveTextDocumentParams(
433+
text_document=TextDocumentIdentifier(
434+
uri=(tmp_path / "header.h").as_uri()
435+
)
436+
)
437+
)
438+
439+
# Hover should detect the change and recompile.
440+
event = client.wait_for_diagnostics(uri)
441+
await client.text_document_hover_async(
442+
HoverParams(text_document=_doc(uri), position=Position(line=0, character=0))
443+
)
444+
await asyncio.wait_for(event.wait(), timeout=60.0)
445+
446+
diags = client.diagnostics.get(uri, [])
447+
assert len(diags) > 0, "Expected diagnostics after didSave on broken header"
448+
449+
134450
async def test_didsave_with_module_deps(client, test_data_dir, tmp_path):
135451
"""didSave on a module file should invalidate CompileGraph dependents."""
136452
src = test_data_dir / "modules" / "save_recompile"

0 commit comments

Comments
 (0)