diff --git a/org.eclipse.xtext.ide.tests/src/org/eclipse/xtext/ide/tests/server/InterleavingRequestManager.java b/org.eclipse.xtext.ide.tests/src/org/eclipse/xtext/ide/tests/server/InterleavingRequestManager.java new file mode 100644 index 00000000000..92c750f74cb --- /dev/null +++ b/org.eclipse.xtext.ide.tests/src/org/eclipse/xtext/ide/tests/server/InterleavingRequestManager.java @@ -0,0 +1,65 @@ +/** + * Copyright (c) 2026 TypeFox GmbH (http://www.typefox.io) and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.xtext.ide.tests.server; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.eclipse.xtext.ide.server.concurrent.RequestManager; +import org.eclipse.xtext.service.OperationCanceledManager; +import org.eclipse.xtext.util.CancelIndicator; +import org.eclipse.xtext.xbase.lib.Functions.Function1; + +import com.google.inject.Inject; + +public class InterleavingRequestManager extends RequestManager { + private final AtomicBoolean gateNextRead = new AtomicBoolean(); + private final CountDownLatch readEntered = new CountDownLatch(1); + private final CountDownLatch continueRead = new CountDownLatch(1); + + @Inject + public InterleavingRequestManager(ExecutorService parallel, OperationCanceledManager operationCanceledManager) { + super(parallel, operationCanceledManager); + } + + public void gateNextRead() { + gateNextRead.set(true); + } + + public boolean awaitReadEntered(long timeout, TimeUnit unit) throws InterruptedException { + return readEntered.await(timeout, unit); + } + + public void continueRead() { + continueRead.countDown(); + } + + @Override + public synchronized CompletableFuture runRead( + Function1 cancellable) { + if (gateNextRead.compareAndSet(true, false)) { + return super.runRead((CancelIndicator cancelIndicator) -> { + readEntered.countDown(); + try { + if (!continueRead.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("Timed out waiting to continue read request."); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(e); + } + return cancellable.apply(cancelIndicator); + }); + } + return super.runRead(cancellable); + } +} diff --git a/org.eclipse.xtext.ide.tests/src/org/eclipse/xtext/ide/tests/server/Rename2ConcurrencyTest.java b/org.eclipse.xtext.ide.tests/src/org/eclipse/xtext/ide/tests/server/Rename2ConcurrencyTest.java new file mode 100644 index 00000000000..e7f01902169 --- /dev/null +++ b/org.eclipse.xtext.ide.tests/src/org/eclipse/xtext/ide/tests/server/Rename2ConcurrencyTest.java @@ -0,0 +1,116 @@ +/** + * Copyright (c) 2017, 2026 TypeFox GmbH (http://www.typefox.io) and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.xtext.ide.tests.server; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +import org.eclipse.lsp4j.ClientCapabilities; +import org.eclipse.lsp4j.InitializeParams; +import org.eclipse.lsp4j.InitializeResult; +import org.eclipse.lsp4j.Position; +import org.eclipse.lsp4j.RenameCapabilities; +import org.eclipse.lsp4j.RenameParams; +import org.eclipse.lsp4j.TextDocumentClientCapabilities; +import org.eclipse.lsp4j.TextDocumentIdentifier; +import org.eclipse.lsp4j.WorkspaceClientCapabilities; +import org.eclipse.lsp4j.WorkspaceEdit; +import org.eclipse.lsp4j.WorkspaceEditCapabilities; +import org.eclipse.xtext.ide.server.concurrent.IRequestManager; +import org.eclipse.xtext.testing.AbstractLanguageServerTest; +import org.eclipse.xtext.util.Modules2; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +import com.google.inject.AbstractModule; +import com.google.inject.Scopes; + +/** + * @author koehnlein - Initial contribution and API + */ +public class Rename2ConcurrencyTest extends AbstractLanguageServerTest { + public Rename2ConcurrencyTest() { + super("fileawaretestlanguage"); + } + + @Test(timeout = 30000) + public void testRenameDoesNotDeadlockWithQueuedWrite() throws Exception { + String model = + "package foo\n" + + "\n" + + "element Foo {\n" + + " ref Foo\n" + + "}\n"; + String file = writeFile("foo/Foo.fileawaretestlanguage", model); + initialize(); + TextDocumentIdentifier identifier = new TextDocumentIdentifier(file); + Position position = new Position(2, 9); + RenameParams params = new RenameParams(identifier, position, "Bar"); + InterleavingRequestManager requestManager = (InterleavingRequestManager) languageServer.getRequestManager(); + + requestManager.gateNextRead(); + CompletableFuture renameResult = languageServer.rename(params); + Assert.assertTrue("Timed out waiting for rename read request to start.", + requestManager.awaitReadEntered(10, TimeUnit.SECONDS)); + CompletableFuture writeResult = requestManager.runWrite(() -> null, + (cancelIndicator, it) -> null); + requestManager.continueRead(); + + WorkspaceEdit workspaceEdit = renameResult.get(10, TimeUnit.SECONDS); + String expectation = + "changes :\n" + + "documentChanges : \n" + + " Foo.fileawaretestlanguage <1> : Bar [[2, 8] .. [2, 11]]\n" + + " Bar [[3, 5] .. [3, 8]]\n"; + assertEquals(expectation.toString(), toExpectation(workspaceEdit)); + writeResult.get(10, TimeUnit.SECONDS); + } + + @Override + protected InitializeResult initialize() { + return super.initialize((InitializeParams params) -> { + ClientCapabilities clientCapabilities = new ClientCapabilities(); + WorkspaceClientCapabilities workspaceClientCapabilities = new WorkspaceClientCapabilities(); + workspaceClientCapabilities.setWorkspaceFolders(true); + WorkspaceEditCapabilities workspaceEditCapabilities = new WorkspaceEditCapabilities(); + workspaceEditCapabilities.setDocumentChanges(true); + workspaceClientCapabilities.setWorkspaceEdit(workspaceEditCapabilities); + clientCapabilities.setWorkspace(workspaceClientCapabilities); + TextDocumentClientCapabilities textDocumentClientCapabilities = new TextDocumentClientCapabilities(); + textDocumentClientCapabilities.setRename(new RenameCapabilities(true, false)); + clientCapabilities.setTextDocument(textDocumentClientCapabilities); + params.setCapabilities(clientCapabilities); + }); + } + + @Override + protected com.google.inject.Module getServerModule() { + com.google.inject.Module defaultModule = super.getServerModule(); + com.google.inject.Module customModule = new AbstractModule() { + @Override + protected void configure() { + bind(IRequestManager.class).to(InterleavingRequestManager.class).in(Scopes.SINGLETON); + } + }; + return Modules2.mixin(defaultModule, customModule); + } + + @Override + @After + public void cleanup() { + try { + super.cleanup(); + } finally { + if (languageServer != null) { + languageServer.getRequestManager().shutdown(); + } + } + } +} diff --git a/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/ChangeConverter2.java b/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/ChangeConverter2.java index d6ccd12097f..73a3bfe7df8 100644 --- a/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/ChangeConverter2.java +++ b/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/ChangeConverter2.java @@ -13,7 +13,6 @@ import java.nio.charset.Charset; import java.util.Arrays; import java.util.List; -import java.util.concurrent.ExecutionException; import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.xmi.XMLResource; @@ -83,14 +82,14 @@ protected void _handleReplacements(IEmfResourceChange change) { String uri = uriExtensions.toUriString(change.getResource().getURI()); change.getResource().save(outputStream, null); String newContent = new String(outputStream.toByteArray(), getCharset(change.getResource())); - access.doRead(uri, (ILanguageServerAccess.Context context) -> { + access.doSyncRead(uri, (ILanguageServerAccess.Context context) -> { Document document = context.getDocument(); Range range = new Range(document.getPosition(0), document.getPosition(document.getContents().length())); TextEdit textEdit = new TextEdit(range, newContent); addTextEdit(uri, document, textEdit); return null; - }).get(); - } catch (InterruptedException | ExecutionException | IOException e) { + }); + } catch (IOException e) { throw Exceptions.sneakyThrow(e); } } @@ -110,24 +109,20 @@ protected String getCharset(Resource resource) { } protected void _handleReplacements(ITextDocumentChange change) { - try { - if (change.getReplacements().size() > 0) { - String uri = uriExtensions.toUriString(change.getNewURI()); - access.doRead(uri, (ILanguageServerAccess.Context context) -> { - Document document = context.getDocument(); - List textEdits = Lists.transform(change.getReplacements(), - (ITextReplacement replacement) -> { - Position start = document.getPosition(replacement.getOffset()); - Position end = document.getPosition(replacement.getEndOffset()); - Range range = new Range(start, end); - return new TextEdit(range, replacement.getReplacementText()); - }); - addTextEdit(uri, document, textEdits.toArray(new TextEdit[textEdits.size()])); - return null; - }).get(); - } - } catch (InterruptedException | ExecutionException e) { - throw Exceptions.sneakyThrow(e); + if (change.getReplacements().size() > 0) { + String uri = uriExtensions.toUriString(change.getNewURI()); + access.doSyncRead(uri, (ILanguageServerAccess.Context context) -> { + Document document = context.getDocument(); + List textEdits = Lists.transform(change.getReplacements(), + (ITextReplacement replacement) -> { + Position start = document.getPosition(replacement.getOffset()); + Position end = document.getPosition(replacement.getEndOffset()); + Range range = new Range(start, end); + return new TextEdit(range, replacement.getReplacementText()); + }); + addTextEdit(uri, document, textEdits.toArray(new TextEdit[textEdits.size()])); + return null; + }); } } diff --git a/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/RenameService2.java b/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/RenameService2.java index 7ac60181378..1270ae4dfac 100644 --- a/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/RenameService2.java +++ b/org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/rename/RenameService2.java @@ -10,7 +10,6 @@ import java.io.FileNotFoundException; import java.util.Objects; -import java.util.concurrent.ExecutionException; import org.apache.log4j.Logger; import org.eclipse.emf.ecore.EAttribute; @@ -87,13 +86,15 @@ public class RenameService2 implements IRenameService2 { @Override public WorkspaceEdit rename(IRenameService2.Options options) { + boolean shouldPrepareRename = false; try { TextDocumentIdentifier textDocument = options.getRenameParams().getTextDocument(); String uri = textDocument.getUri(); ServerRefactoringIssueAcceptor issueAcceptor = issueProvider.get(); - boolean shouldPrepareRename = shouldPrepareRename(options.getLanguageServerAccess()); - return options.getLanguageServerAccess().doRead(uri, (ILanguageServerAccess.Context context) -> { - if (shouldPrepareRename) { + shouldPrepareRename = shouldPrepareRename(options.getLanguageServerAccess()); + boolean prepareRename = shouldPrepareRename; + return options.getLanguageServerAccess().doSyncRead(uri, (ILanguageServerAccess.Context context) -> { + if (prepareRename) { TextDocumentIdentifier identifier = new TextDocumentIdentifier(textDocument.getUri()); Position position = options.getRenameParams().getPosition(); PrepareRenameParams positionParams = new PrepareRenameParams(identifier, position); @@ -131,21 +132,9 @@ public WorkspaceEdit rename(IRenameService2.Options options) { } issueAcceptor.checkSeverity(); return workspaceEdit; - }).exceptionally((Throwable exception) -> { - try { - Throwable rootCause = Throwables.getRootCause(exception); - if (rootCause instanceof FileNotFoundException) { - if (shouldPrepareRename) { - return null; - } - } - throw exception; - } catch (Throwable e) { - throw Exceptions.sneakyThrow(e); - } - }).get(); - } catch (InterruptedException | ExecutionException e) { - throw Exceptions.sneakyThrow(e); + }); + } catch (Throwable exception) { + return handleReadException(exception, shouldPrepareRename); } } @@ -193,11 +182,13 @@ protected EObject getElementWithIdentifierAt(XtextResource xtextResource, int of @Override public Either3 prepareRename(IRenameService2.PrepareRenameOptions options) { + boolean shouldPrepareRename = false; try { String uri = options.getParams().getTextDocument().getUri(); - boolean shouldPrepareRename = shouldPrepareRename(options.getLanguageServerAccess()); - return options.getLanguageServerAccess().doRead(uri, (ILanguageServerAccess.Context context) -> { - if (!shouldPrepareRename) { + shouldPrepareRename = shouldPrepareRename(options.getLanguageServerAccess()); + boolean prepareRename = shouldPrepareRename; + return options.getLanguageServerAccess().doSyncRead(uri, (ILanguageServerAccess.Context context) -> { + if (!prepareRename) { return null; } Resource resource = context.getResource(); @@ -205,22 +196,20 @@ public Either3 prepare PrepareRenameParams params = options.getParams(); CancelIndicator cancelIndicator = options.getCancelIndicator(); return doPrepareRename(resource, document, params, cancelIndicator); - }).exceptionally((Throwable exception) -> { - try { - Throwable rootCause = Throwables.getRootCause(exception); - if (rootCause instanceof FileNotFoundException) { - if (shouldPrepareRename) { - return null; - } - } - throw exception; - } catch (Throwable e) { - throw Exceptions.sneakyThrow(e); - } - }).get(); - } catch (InterruptedException | ExecutionException e) { - throw Exceptions.sneakyThrow(e); + }); + } catch (Throwable exception) { + return handleReadException(exception, shouldPrepareRename); + } + } + + private T handleReadException(Throwable exception, boolean shouldPrepareRename) { + Throwable rootCause = Throwables.getRootCause(exception); + if (rootCause instanceof FileNotFoundException) { + if (shouldPrepareRename) { + return null; + } } + throw Exceptions.sneakyThrow(exception); } protected Either3 doPrepareRename(Resource resource, Document document,