Skip to content

Commit

Permalink
Shut down the ExecutorService to properly clean up the thread
Browse files Browse the repository at this point in the history
Because projects might reuse the Compiler instance for multiple compilations, set a new ExecutorService at the beginning of runInCompilerThread().

Should fix #1607

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=160981960
  • Loading branch information
Dominator008 authored and brad4d committed Jul 6, 2017
1 parent 3065437 commit 7bdbe96
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 22 deletions.
6 changes: 1 addition & 5 deletions src/com/google/javascript/jscomp/Compiler.java
Expand Up @@ -257,7 +257,7 @@ public SourceFile loadSource(String filename) {
DiagnosticType.error("JSC_OPTIMIZE_LOOP_ERROR", DiagnosticType.error("JSC_OPTIMIZE_LOOP_ERROR",
"Exceeded max number of code motion iterations: {0}"); "Exceeded max number of code motion iterations: {0}");


private final CompilerExecutor compilerExecutor = getCompilerExecutor(); private final CompilerExecutor compilerExecutor = new CompilerExecutor();


/** /**
* Logger for the whole com.google.javascript.jscomp domain - * Logger for the whole com.google.javascript.jscomp domain -
Expand Down Expand Up @@ -3441,10 +3441,6 @@ private void renameModules(List<JSModule> newModules, List<JSModule> deserialize
return; return;
} }


protected CompilerExecutor getCompilerExecutor() {
return new CompilerExecutor();
}

/** /**
* Serializable state of the compiler. * Serializable state of the compiler.
*/ */
Expand Down
20 changes: 6 additions & 14 deletions src/com/google/javascript/jscomp/CompilerExecutor.java
Expand Up @@ -30,13 +30,11 @@
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;


/** Run the compiler in a separate thread with a larger stack */ /** Run the compiler in a separate thread with a larger stack */
final class CompilerExecutor { class CompilerExecutor {
// We use many recursive algorithms that use O(d) memory in the depth // We use many recursive algorithms that use O(d) memory in the depth
// of the tree. // of the tree.
private static final long COMPILER_STACK_SIZE = (1 << 24); // About 16MB private static final long COMPILER_STACK_SIZE = (1 << 24); // About 16MB


private final ExecutorService compilerExecutor;

/** /**
* Use a dedicated compiler thread per Compiler instance. * Use a dedicated compiler thread per Compiler instance.
*/ */
Expand All @@ -47,15 +45,6 @@ final class CompilerExecutor {


private int timeout = 0; private int timeout = 0;


CompilerExecutor() {
this(getDefaultExecutorService());
}

@GwtIncompatible("java.util.concurrent.ExecutorService")
CompilerExecutor(ExecutorService compilerExecutorService) {
this.compilerExecutor = compilerExecutorService;
}

/** /**
* Under JRE 1.6, the JS Compiler overflows the stack when running on some * Under JRE 1.6, the JS Compiler overflows the stack when running on some
* large or complex JS code. When threads are available, we run all compile * large or complex JS code. When threads are available, we run all compile
Expand All @@ -65,7 +54,7 @@ final class CompilerExecutor {
* (which is what -Xss does). * (which is what -Xss does).
*/ */
@GwtIncompatible("java.util.concurrent.ExecutorService") @GwtIncompatible("java.util.concurrent.ExecutorService")
static ExecutorService getDefaultExecutorService() { ExecutorService getExecutorService() {
return Executors.newSingleThreadExecutor(new ThreadFactory() { return Executors.newSingleThreadExecutor(new ThreadFactory() {
@Override @Override
public Thread newThread(Runnable r) { public Thread newThread(Runnable r) {
Expand All @@ -86,6 +75,7 @@ void setTimeout(int timeout) {


@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
<T> T runInCompilerThread(final Callable<T> callable, final boolean dumpTraceReport) { <T> T runInCompilerThread(final Callable<T> callable, final boolean dumpTraceReport) {
ExecutorService executor = getExecutorService();
T result = null; T result = null;
final Throwable[] exception = new Throwable[1]; final Throwable[] exception = new Throwable[1];


Expand Down Expand Up @@ -118,14 +108,16 @@ public T call() {
} }
}; };


Future<T> future = compilerExecutor.submit(bootCompilerThread); Future<T> future = executor.submit(bootCompilerThread);
if (timeout > 0) { if (timeout > 0) {
result = future.get(timeout, TimeUnit.SECONDS); result = future.get(timeout, TimeUnit.SECONDS);
} else { } else {
result = future.get(); result = future.get();
} }
} catch (InterruptedException | TimeoutException | ExecutionException e) { } catch (InterruptedException | TimeoutException | ExecutionException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} finally {
executor.shutdown();
} }
} else { } else {
try { try {
Expand Down
Expand Up @@ -20,9 +20,6 @@


/** GWT compatible replacement for {@code CompilerExecutor} */ /** GWT compatible replacement for {@code CompilerExecutor} */
final class CompilerExecutor { final class CompilerExecutor {

CompilerExecutor() {}

<T> T runInCompilerThread(Callable<T> callable, boolean dumpTraceReport) { <T> T runInCompilerThread(Callable<T> callable, boolean dumpTraceReport) {
try { try {
return callable.call(); return callable.call();
Expand Down

0 comments on commit 7bdbe96

Please sign in to comment.