Merge remote-tracking branch 'origin/copilot/sub-pr-3724' into fix/possible-race-on-did-close

Author: nixel2007

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

@@ -140,6 +140,7 @@
 public class BSLTextDocumentService implements TextDocumentService, ProtocolExtension {
 
   private static final long AWAIT_CLOSE = 30;
+  private static final long AWAIT_FORCE_TERMINATION = 1;
 
   private final ServerContext context;
   private final LanguageServerConfiguration configuration;
@@ -532,15 +533,31 @@ public void didClose(DidCloseTextDocumentParams params) {
           docExecutor.shutdownNow();
           // Must wait for worker thread to finish even after shutdownNow,
           // because finally block in worker may still be executing flushPendingChanges
-          docExecutor.awaitTermination(1, TimeUnit.SECONDS);
+          boolean terminated = docExecutor.awaitTermination(AWAIT_FORCE_TERMINATION, TimeUnit.SECONDS);
+          if (!terminated) {
+            LOGGER.warn(
+              "Document executor for URI {} did not terminate within the additional timeout after shutdownNow()",
+              uri
+            );
+          }
         }
       } catch (InterruptedException e) {
         docExecutor.shutdownNow();
         // Wait briefly for worker to finish after interrupt
         try {
-          docExecutor.awaitTermination(1, TimeUnit.SECONDS);
+          boolean terminated = docExecutor.awaitTermination(AWAIT_FORCE_TERMINATION, TimeUnit.SECONDS);
+          if (!terminated) {
+            LOGGER.warn(
+              "Document executor for URI {} did not terminate within {} seconds after interrupt during document close",
+              uri,
+              AWAIT_FORCE_TERMINATION
+            );
+          }
         } catch (InterruptedException ignored) {
-          // Already interrupted, just restore flag
+          LOGGER.warn(
+            "Interrupted again while waiting for document executor for URI {} to terminate after shutdownNow",
+            uri
+          );
         }
         Thread.currentThread().interrupt();
       }

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

@@ -201,6 +201,93 @@ void didClose() {
     textDocumentService.didClose(params);
   }
 
+  @Test
+  void didCloseWithPendingChanges() throws IOException {
+    // given - open a document and make changes
+    var textDocumentItem = getTextDocumentItem();
+    var didOpenParams = new DidOpenTextDocumentParams(textDocumentItem);
+    textDocumentService.didOpen(didOpenParams);
+
+    var documentContext = serverContext.getDocumentUnsafe(textDocumentItem.getUri());
+    assertThat(documentContext).isNotNull();
+    assertThat(serverContext.isDocumentOpened(documentContext)).isTrue();
+
+    // when - submit multiple changes rapidly and then close immediately
+    var params = new DidChangeTextDocumentParams();
+    var uri = textDocumentItem.getUri();
+    
+    for (int i = 0; i < 5; i++) {
+      params.setTextDocument(new VersionedTextDocumentIdentifier(uri, 2 + i));
+      var range = Ranges.create(0, 0, 0, 0);
+      var changeEvent = new TextDocumentContentChangeEvent(range, "// Change " + i + "\n");
+      List<TextDocumentContentChangeEvent> contentChanges = new ArrayList<>();
+      contentChanges.add(changeEvent);
+      params.setContentChanges(contentChanges);
+      textDocumentService.didChange(params);
+    }
+
+    // then - close should wait for pending changes to complete
+    var closeParams = new DidCloseTextDocumentParams();
+    closeParams.setTextDocument(new TextDocumentIdentifier(uri));
+    textDocumentService.didClose(closeParams);
+
+    // verify the document is closed
+    assertThat(serverContext.isDocumentOpened(documentContext)).isFalse();
+  }
+
+  @Test
+  void didCloseDuringActiveChange() throws IOException {
+    // given - open a document
+    var textDocumentItem = getTextDocumentItem();
+    var didOpenParams = new DidOpenTextDocumentParams(textDocumentItem);
+    textDocumentService.didOpen(didOpenParams);
+
+    var documentContext = serverContext.getDocumentUnsafe(textDocumentItem.getUri());
+    assertThat(documentContext).isNotNull();
+    assertThat(serverContext.isDocumentOpened(documentContext)).isTrue();
+
+    // when - submit a change
+    var params = new DidChangeTextDocumentParams();
+    var uri = textDocumentItem.getUri();
+    params.setTextDocument(new VersionedTextDocumentIdentifier(uri, 2));
+    var range = Ranges.create(0, 0, 0, 0);
+    var changeEvent = new TextDocumentContentChangeEvent(range, "// New content\n");
+    List<TextDocumentContentChangeEvent> contentChanges = new ArrayList<>();
+    contentChanges.add(changeEvent);
+    params.setContentChanges(contentChanges);
+    textDocumentService.didChange(params);
+
+    // then - close immediately while change may still be processing
+    var closeParams = new DidCloseTextDocumentParams();
+    closeParams.setTextDocument(new TextDocumentIdentifier(uri));
+    textDocumentService.didClose(closeParams);
+
+    // verify the document is closed
+    assertThat(serverContext.isDocumentOpened(documentContext)).isFalse();
+  }
+
+  @Test
+  void didCloseAwaitTerminationCompletes() throws IOException {
+    // given - open a document
+    var textDocumentItem = getTextDocumentItem();
+    var didOpenParams = new DidOpenTextDocumentParams(textDocumentItem);
+    textDocumentService.didOpen(didOpenParams);
+
+    var uri = textDocumentItem.getUri();
+    var documentContext = serverContext.getDocumentUnsafe(uri);
+    assertThat(documentContext).isNotNull();
+    assertThat(serverContext.isDocumentOpened(documentContext)).isTrue();
+
+    // when - close the document (which should wait for executor to terminate)
+    var closeParams = new DidCloseTextDocumentParams();
+    closeParams.setTextDocument(new TextDocumentIdentifier(uri));
+    textDocumentService.didClose(closeParams);
+
+    // then - verify the document is properly closed
+    // The close should complete successfully even if executor needs time to terminate
+    assertThat(serverContext.isDocumentOpened(documentContext)).isFalse();
+  }
+
   @Test
   void didClosePublishesEmptyDiagnosticsWhenClientDoesNotSupportPullDiagnostics() throws IOException {
     // given - open a document