Skip to content

Commit 3f57e26

Browse files
committed
Use a separate ExecutorService in AsyncCompletionProposalPopup
This is necessary in order to effectively cancel all running futures in the popup (when one closes the popup).
1 parent f1633b7 commit 3f57e26

File tree

3 files changed

+107
-2
lines changed

3 files changed

+107
-2
lines changed

bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,6 @@ Require-Bundle:
4040
Import-Package: com.ibm.icu.text
4141
Bundle-RequiredExecutionEnvironment: JavaSE-17
4242
Automatic-Module-Name: org.eclipse.jface.text
43+
Bundle-Activator: org.eclipse.jface.text.Activator
44+
Bundle-ActivationPolicy: lazy
4345
Require-Capability: eclipse.swt;filter:="(image.format=svg)"
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package org.eclipse.jface.text;
2+
3+
import java.util.concurrent.ExecutorService;
4+
import java.util.concurrent.LinkedBlockingQueue;
5+
import java.util.concurrent.ThreadFactory;
6+
import java.util.concurrent.ThreadPoolExecutor;
7+
import java.util.concurrent.TimeUnit;
8+
import java.util.concurrent.atomic.AtomicInteger;
9+
10+
import org.osgi.framework.BundleActivator;
11+
import org.osgi.framework.BundleContext;
12+
13+
/**
14+
* @since 3.29
15+
*/
16+
public class Activator implements BundleActivator {
17+
/**
18+
* The identifier of the descriptor of this plugin in plugin.xml.
19+
*/
20+
public static final String ID= "org.eclipse.jface.text"; //$NON-NLS-1$
21+
22+
private static Activator activator;
23+
24+
private ExecutorService executor;
25+
26+
@Override
27+
public void start(BundleContext context) {
28+
activator= this;
29+
}
30+
31+
@Override
32+
public void stop(BundleContext context) {
33+
activator= null;
34+
if (executor != null) {
35+
executor.shutdownNow();
36+
executor= null;
37+
}
38+
}
39+
40+
public static ExecutorService getExecutor() {
41+
activator.createExecutor();
42+
return activator.executor;
43+
}
44+
45+
private void createExecutor() {
46+
if (activator.executor != null)
47+
return;
48+
49+
executor= new ThreadPoolExecutor(
50+
Runtime.getRuntime().availableProcessors(),
51+
Runtime.getRuntime().availableProcessors(),
52+
3L, TimeUnit.SECONDS,
53+
new LinkedBlockingQueue<>(),
54+
new ThreadFactory() {
55+
AtomicInteger count= new AtomicInteger(1);
56+
57+
@Override
58+
public Thread newThread(Runnable r) {
59+
// Name the threads numerically for better debugging.
60+
Thread t= new Thread(r, ID + "-worker-" + count.getAndIncrement()); //$NON-NLS-1$
61+
62+
// No need to keep the JVM running just because of the completion proposals
63+
t.setDaemon(true);
64+
return t;
65+
}
66+
});
67+
}
68+
}

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/AsyncCompletionProposalPopup.java

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,11 @@
2323
import java.util.List;
2424
import java.util.Optional;
2525
import java.util.Set;
26+
import java.util.concurrent.Callable;
2627
import java.util.concurrent.CompletableFuture;
2728
import java.util.concurrent.ExecutionException;
29+
import java.util.concurrent.ExecutorService;
30+
import java.util.concurrent.Future;
2831
import java.util.concurrent.TimeUnit;
2932
import java.util.concurrent.TimeoutException;
3033
import java.util.concurrent.atomic.AtomicInteger;
@@ -43,6 +46,7 @@
4346

4447
import org.eclipse.jface.contentassist.IContentAssistSubjectControl;
4548

49+
import org.eclipse.jface.text.Activator;
4650
import org.eclipse.jface.text.BadLocationException;
4751
import org.eclipse.jface.text.DocumentEvent;
4852
import org.eclipse.jface.text.IDocument;
@@ -372,7 +376,7 @@ protected List<CompletableFuture<List<ICompletionProposal>>> buildCompletionFutu
372376
}
373377
List<CompletableFuture<List<ICompletionProposal>>> futures = new ArrayList<>(processors.size());
374378
for (IContentAssistProcessor processor : processors) {
375-
futures.add(CompletableFuture.supplyAsync(() -> {
379+
futures.add(submitInterruptible(() -> {
376380
AtomicReference<List<ICompletionProposal>> result= new AtomicReference<>();
377381
SafeRunner.run(() -> {
378382
ICompletionProposal[] proposals= processor.computeCompletionProposals(fViewer, invocationOffset);
@@ -389,11 +393,42 @@ protected List<CompletableFuture<List<ICompletionProposal>>> buildCompletionFutu
389393
return Collections.emptyList();
390394
}
391395
return proposals;
392-
}));
396+
}, Activator.getExecutor()));
393397
}
394398
return futures;
395399
}
396400

401+
/**
402+
* Submit a task in such a way that it actually reacts to cancellation (i.e. calls to
403+
* {@code future.cancel(true)})
404+
*
405+
* @param executor Do not use the common pool here since that one does not cancel (interrupts)
406+
* worker threads
407+
* @return an interruptible future.
408+
*/
409+
private static <T> CompletableFuture<T> submitInterruptible(
410+
Callable<T> task, ExecutorService executor) {
411+
412+
CompletableFuture<T> cf= new CompletableFuture<>();
413+
414+
Future<?> ft= executor.submit(() -> {
415+
try {
416+
cf.complete(task.call());
417+
} catch (Exception e) {
418+
cf.completeExceptionally(e);
419+
}
420+
});
421+
422+
// make canceling the CF also cancel the FutureTask
423+
cf.whenComplete((r, t) -> {
424+
if (cf.isCancelled()) {
425+
ft.cancel(true); // this actually interrupts
426+
}
427+
});
428+
429+
return cf;
430+
}
431+
397432
private String getTokenContentType(int invocationOffset) throws BadLocationException {
398433
if (fContentAssistSubjectControl != null) {
399434
IDocument document= fContentAssistSubjectControl.getDocument();

0 commit comments

Comments
 (0)