Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
Ensure resources used during compilation of java code are released.
Browse files Browse the repository at this point in the history
Summary:
Turns out that we were opening a `ZipFile` but never calling `close()`
on it when compiling java code in memory from a source zip. This was
causing references to the `ZipEntry` instances to be kept in perm gen
space, ultimately causing it to be exhausted and making Buck fall over,
complaining wildly about that.

Test Plan:
for i in 1 2 3 4 5 6 7 8 9 10; do rm -rf buck-out; buck build buck; done
Attach jvisualvm, look at permgen space. See that's it's not totally
crazy after forcing a gc.
Look at a heapdump and look for ZipEntry instances. There should be none.
  • Loading branch information
shs96c authored and oconnor663 committed Aug 18, 2014
1 parent 44ba73a commit 63e0e52
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 5 deletions.
36 changes: 32 additions & 4 deletions src/com/facebook/buck/java/JavacInMemoryStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.common.base.Splitter;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.io.Files;
Expand All @@ -58,6 +59,7 @@
import javax.tools.Diagnostic;
import javax.tools.DiagnosticCollector;
import javax.tools.JavaCompiler;
import javax.tools.JavaFileManager;
import javax.tools.JavaFileObject;
import javax.tools.StandardJavaFileManager;
import javax.tools.ToolProvider;
Expand Down Expand Up @@ -127,11 +129,12 @@ protected int buildWithClasspath(ExecutionContext context, Set<Path> buildClassp
Preconditions.checkNotNull(compiler,
"If using JRE instead of JDK, ToolProvider.getSystemJavaCompiler() may be null.");
StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, null, null);
Iterable<? extends JavaFileObject> compilationUnits;
Iterable<? extends JavaFileObject> compilationUnits = ImmutableSet.of();
try {
compilationUnits = createCompilationUnits(
fileManager, context.getProjectFilesystem().getAbsolutifier());
} catch (IOException e) {
close(fileManager, compilationUnits, null);
e.printStackTrace(context.getStdErr());
return 1;
}
Expand All @@ -145,6 +148,7 @@ protected int buildWithClasspath(ExecutionContext context, Set<Path> buildClassp
Iterables.transform(javaSourceFilePaths, Functions.toStringFunction()),
pathToSrcsList.get());
} catch (IOException e) {
close(fileManager, compilationUnits, null);
context.logError(e,
"Cannot write list of .java files to compile to %s file! Terminating compilation.",
pathToSrcsList.get());
Expand Down Expand Up @@ -178,9 +182,7 @@ protected int buildWithClasspath(ExecutionContext context, Set<Path> buildClassp
// Invoke the compilation and inspect the result.
isSuccess = compilationTask.call();
} finally {
if (bundle != null) {
bundle.close();
}
close(fileManager, compilationUnits, bundle);
}

if (isSuccess) {
Expand Down Expand Up @@ -220,6 +222,32 @@ protected int buildWithClasspath(ExecutionContext context, Set<Path> buildClassp
}
}

private void close(
JavaFileManager fileManager,
Iterable<? extends JavaFileObject> compilationUnits,
@Nullable ProcessorBundle bundle) {
try {
fileManager.close();
} catch (IOException e) {
LOG.warn(e, "Unable to close java filemanager. We may be leaking memory.");
}

for (JavaFileObject unit : compilationUnits) {
if (!(unit instanceof ZipEntryJavaFileObject)) {
continue;
}
try {
((ZipEntryJavaFileObject) unit).close();
} catch (IOException e) {
LOG.warn(e, "Unable to close zipfile. We may be leaking memory.");
}
}

if (bundle != null) {
bundle.close();
}
}

private ProcessorBundle prepareProcessors(@Nullable BuildTarget target, List<String> options) {
String processorClassPath = null;
String processorNames = null;
Expand Down
11 changes: 10 additions & 1 deletion src/com/facebook/buck/java/ZipEntryJavaFileObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.base.Preconditions;
import com.google.common.io.CharStreams;

import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
Expand All @@ -33,7 +34,7 @@
import javax.tools.JavaFileObject;
import javax.tools.SimpleJavaFileObject;

class ZipEntryJavaFileObject extends SimpleJavaFileObject {
class ZipEntryJavaFileObject extends SimpleJavaFileObject implements Closeable {

private final ZipFile zipFile;
private final ZipEntry zipEntry;
Expand Down Expand Up @@ -78,6 +79,14 @@ private synchronized String getContentsAsString() {
return contents;
}

/**
* Closes the {@link ZipFile} this entry was loaded from. Use with care.
*/
@Override
public void close() throws IOException {
zipFile.close();
}

@Override
public CharSequence getCharContent(boolean ignoreEncodingErrors) {
return getContentsAsString();
Expand Down

0 comments on commit 63e0e52

Please sign in to comment.