Skip to content

Commit 4b3c193

Browse files
authored
Merge pull request #3724 from 1c-syntax/fix/possible-race-on-did-close
Исправлена потенциальная гонка при закрытии документа в BSLTextDocumentService.
2 parents c6986b0 + 9f00ce2 commit 4b3c193

File tree

2 files changed

+113
-0
lines changed

2 files changed

+113
-0
lines changed

src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@
140140
public class BSLTextDocumentService implements TextDocumentService, ProtocolExtension {
141141

142142
private static final long AWAIT_CLOSE = 30;
143+
private static final long AWAIT_FORCE_TERMINATION = 1;
143144

144145
private final ServerContext context;
145146
private final LanguageServerConfiguration configuration;
@@ -530,9 +531,34 @@ public void didClose(DidCloseTextDocumentParams params) {
530531
// Wait for all queued changes to complete (with timeout to avoid hanging)
531532
if (!docExecutor.awaitTermination(AWAIT_CLOSE, TimeUnit.SECONDS)) {
532533
docExecutor.shutdownNow();
534+
// Must wait for worker thread to finish even after shutdownNow,
535+
// because finally block in worker may still be executing flushPendingChanges
536+
boolean terminated = docExecutor.awaitTermination(AWAIT_FORCE_TERMINATION, TimeUnit.SECONDS);
537+
if (!terminated) {
538+
LOGGER.warn(
539+
"Document executor for URI {} did not terminate within the additional timeout after shutdownNow()",
540+
uri
541+
);
542+
}
533543
}
534544
} catch (InterruptedException e) {
535545
docExecutor.shutdownNow();
546+
// Wait briefly for worker to finish after interrupt
547+
try {
548+
boolean terminated = docExecutor.awaitTermination(AWAIT_FORCE_TERMINATION, TimeUnit.SECONDS);
549+
if (!terminated) {
550+
LOGGER.warn(
551+
"Document executor for URI {} did not terminate within {} seconds after interrupt during document close",
552+
uri,
553+
AWAIT_FORCE_TERMINATION
554+
);
555+
}
556+
} catch (InterruptedException ignored) {
557+
LOGGER.warn(
558+
"Interrupted again while waiting for document executor for URI {} to terminate after shutdownNow",
559+
uri
560+
);
561+
}
536562
Thread.currentThread().interrupt();
537563
}
538564
}

src/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,93 @@ void didClose() {
201201
textDocumentService.didClose(params);
202202
}
203203

204+
@Test
205+
void didCloseWithPendingChanges() throws IOException {
206+
// given - open a document and make changes
207+
var textDocumentItem = getTextDocumentItem();
208+
var didOpenParams = new DidOpenTextDocumentParams(textDocumentItem);
209+
textDocumentService.didOpen(didOpenParams);
210+
211+
var documentContext = serverContext.getDocumentUnsafe(textDocumentItem.getUri());
212+
assertThat(documentContext).isNotNull();
213+
assertThat(serverContext.isDocumentOpened(documentContext)).isTrue();
214+
215+
// when - submit multiple changes rapidly and then close immediately
216+
var params = new DidChangeTextDocumentParams();
217+
var uri = textDocumentItem.getUri();
218+
219+
for (int i = 0; i < 5; i++) {
220+
params.setTextDocument(new VersionedTextDocumentIdentifier(uri, 2 + i));
221+
var range = Ranges.create(0, 0, 0, 0);
222+
var changeEvent = new TextDocumentContentChangeEvent(range, "// Change " + i + "\n");
223+
List<TextDocumentContentChangeEvent> contentChanges = new ArrayList<>();
224+
contentChanges.add(changeEvent);
225+
params.setContentChanges(contentChanges);
226+
textDocumentService.didChange(params);
227+
}
228+
229+
// then - close should wait for pending changes to complete
230+
var closeParams = new DidCloseTextDocumentParams();
231+
closeParams.setTextDocument(new TextDocumentIdentifier(uri));
232+
textDocumentService.didClose(closeParams);
233+
234+
// verify the document is closed
235+
assertThat(serverContext.isDocumentOpened(documentContext)).isFalse();
236+
}
237+
238+
@Test
239+
void didCloseDuringActiveChange() throws IOException {
240+
// given - open a document
241+
var textDocumentItem = getTextDocumentItem();
242+
var didOpenParams = new DidOpenTextDocumentParams(textDocumentItem);
243+
textDocumentService.didOpen(didOpenParams);
244+
245+
var documentContext = serverContext.getDocumentUnsafe(textDocumentItem.getUri());
246+
assertThat(documentContext).isNotNull();
247+
assertThat(serverContext.isDocumentOpened(documentContext)).isTrue();
248+
249+
// when - submit a change
250+
var params = new DidChangeTextDocumentParams();
251+
var uri = textDocumentItem.getUri();
252+
params.setTextDocument(new VersionedTextDocumentIdentifier(uri, 2));
253+
var range = Ranges.create(0, 0, 0, 0);
254+
var changeEvent = new TextDocumentContentChangeEvent(range, "// New content\n");
255+
List<TextDocumentContentChangeEvent> contentChanges = new ArrayList<>();
256+
contentChanges.add(changeEvent);
257+
params.setContentChanges(contentChanges);
258+
textDocumentService.didChange(params);
259+
260+
// then - close immediately while change may still be processing
261+
var closeParams = new DidCloseTextDocumentParams();
262+
closeParams.setTextDocument(new TextDocumentIdentifier(uri));
263+
textDocumentService.didClose(closeParams);
264+
265+
// verify the document is closed
266+
assertThat(serverContext.isDocumentOpened(documentContext)).isFalse();
267+
}
268+
269+
@Test
270+
void didCloseAwaitTerminationCompletes() throws IOException {
271+
// given - open a document
272+
var textDocumentItem = getTextDocumentItem();
273+
var didOpenParams = new DidOpenTextDocumentParams(textDocumentItem);
274+
textDocumentService.didOpen(didOpenParams);
275+
276+
var uri = textDocumentItem.getUri();
277+
var documentContext = serverContext.getDocumentUnsafe(uri);
278+
assertThat(documentContext).isNotNull();
279+
assertThat(serverContext.isDocumentOpened(documentContext)).isTrue();
280+
281+
// when - close the document (which should wait for executor to terminate)
282+
var closeParams = new DidCloseTextDocumentParams();
283+
closeParams.setTextDocument(new TextDocumentIdentifier(uri));
284+
textDocumentService.didClose(closeParams);
285+
286+
// then - verify the document is properly closed
287+
// The close should complete successfully even if executor needs time to terminate
288+
assertThat(serverContext.isDocumentOpened(documentContext)).isFalse();
289+
}
290+
204291
@Test
205292
void didClosePublishesEmptyDiagnosticsWhenClientDoesNotSupportPullDiagnostics() throws IOException {
206293
// given - open a document

0 commit comments

Comments
 (0)