Skip to content

Commit

Permalink
Fix FileDescriptor leaks
Browse files Browse the repository at this point in the history 
ProjectManager
- add a close method that closes the repo (and implement AutoCloseable)
- close build-instance repo after build
- delete the workDir with OVERRIDE_READ_ONLY
BuildResult
- close ProjectManager after build so the repo is closed
  • Loading branch information
@LeeWorrall
LeeWorrall committed Dec 19, 2020
1 parent 6946220 commit 8c13e90
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 59 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Deploying to Nexus

**This section applies only to project owners**

The JARs must be signed and you must have access to upload to Sonatype, so you need a GPG key and a Sonatype login.
The JARs must be signed, and you must have access to upload to Sonatype, so you need a GPG key and a Sonatype login.
The passwords for this should be in your Maven `settings.xml` with the following config:

<settings>
Expand Down
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Restabuild
----------

Your self-hosted build server that let's you quickly build projects from a RESTful interface only.
Your self-hosted build server that lets you quickly build projects from a RESTful interface only.
Simply place `build.sh` (or `build.bat` if the build server is Windows) into the root of your project
and then `POST` to `/restabuild/api/v1/builds` with `gitUrl` as a form parameter. The web UI gives
more detail on the API along with sample curl commands.
Expand All @@ -16,7 +16,7 @@ Configuration
-------------

See `sample-config.properties` for configuration information. Each setting can be specified
as an environment variable, a java system property, or in a properties file who's path is
as an environment variable, a java system property, or in a properties file whose path is
specified as a command line argument.

Running
Expand Down
6 changes: 3 additions & 3 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.9</version>
<version>3.11</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<version>2.6</version>
<version>2.8.0</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
Expand All @@ -83,7 +83,7 @@
<dependency>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
<version>1.13</version>
<version>1.15</version>
</dependency>

<dependency>
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/danielflower/restabuild/build/BuildResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ public class BuildResult {
private volatile BuildState state = BuildState.QUEUED;
private final GitRepo gitRepo;
private volatile StringBuffer buildLog = new StringBuffer();
private File buildLogFile;
private final File buildLogFile;
private final List<StringListener> logListeners = new CopyOnWriteArrayList<>();
public final long queueStart = System.currentTimeMillis();
private long buildStart = -1;
private long buildComplete = -1;
private String commitIDBeforeBuild;
private String commitIDAfterBuild;
private List<String> createdTags;
private String buildParam;
private final String buildParam;
private final Map<String, String> environment;

public BuildResult(FileSandbox sandbox, GitRepo gitRepo, String buildParam, String id, Map<String, String> environment) {
Expand Down Expand Up @@ -86,8 +86,7 @@ public void run(int buildTimeout) throws Exception {
buildStart = System.currentTimeMillis();
try (FileWriter logFileWriter = new FileWriter(buildLogFile);
Writer writer = new MultiWriter(logFileWriter)) {
try {
ProjectManager pm = ProjectManager.create(gitRepo.url, sandbox, writer);
try (ProjectManager pm = ProjectManager.create(gitRepo.url, sandbox, writer)) {
extendedBuildState = pm.build(writer, gitRepo.branch, buildParam, buildTimeout, environment);
newState = extendedBuildState.buildState;
} catch (Exception ex) {
Expand Down
116 changes: 67 additions & 49 deletions src/main/java/com/danielflower/restabuild/build/ProjectManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import io.muserver.Mutils;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.exec.CommandLine;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.file.DeleteOption;
import org.apache.commons.io.file.PathUtils;
import org.apache.commons.io.file.StandardDeleteOption;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.SystemUtils;
import org.eclipse.jgit.api.Git;
Expand All @@ -29,7 +31,7 @@

import static com.danielflower.restabuild.FileSandbox.dirPath;

public class ProjectManager {
public class ProjectManager implements AutoCloseable {
static {
JSch.setConfig("StrictHostKeyChecking", "no");
}
Expand Down Expand Up @@ -93,77 +95,85 @@ static class ExtendedBuildState {
public final String commitIDBeforeBuild;
public final String commitIDAfterBuild;
public final List<String> tagsAdded;
ExtendedBuildState(BuildState buildState, String commitIDBeforeBuild, String commitIDAfterBuild, List<String> tagsAdded) {
public final File workDir;
ExtendedBuildState(BuildState buildState, String commitIDBeforeBuild, String commitIDAfterBuild, List<String> tagsAdded, File workDir) {
this.buildState = buildState;
this.commitIDBeforeBuild = commitIDBeforeBuild;
this.commitIDAfterBuild = commitIDAfterBuild;
this.tagsAdded = tagsAdded;
this.workDir = workDir;
}
}

public ExtendedBuildState build(Writer outputHandler, String branch, String buildParam, int buildTimeout, Map<String, String> environment) throws Exception {
doubleLog(outputHandler, "Fetching latest changes from git...");
File workDir = pullFromGitAndCopyWorkingCopyToNewDir(outputHandler, branch);
doubleLog(outputHandler, "Created new instance in " + dirPath(workDir));

Git git = Git.open(workDir);


Ref headBefore = git.getRepository().exactRef("HEAD");
Ref headAfter = headBefore;
ObjectId beforeCommitID = headBefore.getObjectId();
List<String> newTags = new ArrayList<>();
List<String> tagsBefore = getTagsAt(git, beforeCommitID);

File f = new File(workDir, buildFile);
BuildState result;
if (!f.isFile()) {
outputHandler.write("Please place a file called " + buildFile + " in the root of your repo");
result = BuildState.FAILURE;
} else {

CommandLine command;
if (SystemUtils.IS_OS_WINDOWS) {
command = new CommandLine(f);
final File workDir;
final ExtendedBuildState extendedBuildState;
try (Git git = pullFromGitAndCopyWorkingCopyToNewDir(outputHandler, branch)) {
workDir = git.getRepository().getWorkTree();
doubleLog(outputHandler, "Created new instance in " + dirPath(workDir));

Ref headBefore = git.getRepository().exactRef("HEAD");
Ref headAfter = headBefore;
ObjectId beforeCommitID = headBefore.getObjectId();
List<String> newTags = new ArrayList<>();
List<String> tagsBefore = getTagsAt(git, beforeCommitID);

File f = new File(workDir, buildFile);
BuildState result;
if (!f.isFile()) {
outputHandler.write("Please place a file called " + buildFile + " in the root of your repo");
result = BuildState.FAILURE;
} else {
command = new CommandLine("bash")
.addArgument("-x")
.addArgument(f.getName());
}
if (StringUtils.isNoneBlank(buildParam)) {
command.addArguments(buildParam);
}
ProcessStarter processStarter = new ProcessStarter(outputHandler);
result = processStarter.run(outputHandler, command, workDir, TimeUnit.MINUTES.toMillis(buildTimeout), environment);

headAfter = git.getRepository().exactRef("HEAD");
CommandLine command;
if (SystemUtils.IS_OS_WINDOWS) {
command = new CommandLine(f);
} else {
command = new CommandLine("bash")
.addArgument("-x")
.addArgument(f.getName());
}
if (StringUtils.isNoneBlank(buildParam)) {
command.addArguments(buildParam);
}
ProcessStarter processStarter = new ProcessStarter(outputHandler);
result = processStarter.run(outputHandler, command, workDir, TimeUnit.MINUTES.toMillis(buildTimeout), environment);

headAfter = git.getRepository().exactRef("HEAD");

try (RevWalk walk = new RevWalk(git.getRepository())) {
walk.markStart(walk.parseCommit(headAfter.getObjectId()));
walk.markUninteresting(walk.parseCommit(headBefore.getObjectId()));
for (RevCommit commit : walk) {
getTagsAt(git, commit.getId()).forEach(s -> newTags.add(0, s));
try (RevWalk walk = new RevWalk(git.getRepository())) {
walk.markStart(walk.parseCommit(headAfter.getObjectId()));
walk.markUninteresting(walk.parseCommit(headBefore.getObjectId()));
for (RevCommit commit : walk) {
getTagsAt(git, commit.getId()).forEach(s -> newTags.add(0, s));
}
}
getTagsAt(git, beforeCommitID).forEach(s -> newTags.add(0, s));
}
getTagsAt(git, beforeCommitID).forEach(s -> newTags.add(0, s));
}

FileUtils.deleteQuietly(workDir);
newTags.removeAll(tagsBefore);

for (String existingTag : tagsBefore) {
newTags.remove(existingTag);
extendedBuildState = new ExtendedBuildState(result, beforeCommitID.name(), headAfter.getObjectId().name(), Collections.unmodifiableList(newTags), workDir);
}

return new ExtendedBuildState(result, beforeCommitID.name(), headAfter.getObjectId().name(), Collections.unmodifiableList(newTags));
deleteDirectoryQuietly(workDir, StandardDeleteOption.OVERRIDE_READ_ONLY);

return extendedBuildState;
}

public void close() {
if (git != null) {
git.close();
}
}

private File pullFromGitAndCopyWorkingCopyToNewDir(Writer writer, String branch) throws GitAPIException, IOException {
private Git pullFromGitAndCopyWorkingCopyToNewDir(Writer writer, String branch) throws GitAPIException, IOException {
git.fetch().setRemote("origin").setProgressMonitor(new TextProgressMonitor(writer)).call();
return copyToNewInstanceDirAndSwitchBranch(branch);
}

private File copyToNewInstanceDirAndSwitchBranch(String branch) throws GitAPIException, IOException {
private Git copyToNewInstanceDirAndSwitchBranch(String branch) throws GitAPIException, IOException {
File dest = new File(instanceDir, String.valueOf(System.currentTimeMillis()));
if (!dest.mkdir()) {
throw new RuntimeException("Could not create " + dirPath(dest));
Expand All @@ -182,7 +192,7 @@ private File copyToNewInstanceDirAndSwitchBranch(String branch) throws GitAPIExc
}

setRemoteOriginUrl(copy.getRepository(), gitUrl);
return dest;
return copy;
}

private static List<String> getTagsAt(Git git, ObjectId commitID) throws GitAPIException {
Expand Down Expand Up @@ -212,4 +222,12 @@ private static void doubleLog(Writer writer, String message) {
log.info(message);
ProcessStarter.writeLine(writer, message);
}

private static void deleteDirectoryQuietly(File workDir, DeleteOption... options) {
try {
PathUtils.deleteDirectory(workDir.toPath(), options);
} catch (IOException e) {
log.debug("Failed to delete {}", workDir, e);
}
}
}
2 changes: 2 additions & 0 deletions src/test/java/com/danielflower/restabuild/build/ProjectManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ public void canBuildProjectsAndPickUpChangesFromMasterBranch() throws Exception
assertThat(buildLog.toString(), extendedBuildState.buildState, is(BuildState.SUCCESS));
assertThat(buildLog.toString(), containsString("BUILD SUCCESS"));
assertThat(extendedBuildState.tagsAdded, contains("my-maven-app-1.0.0"));
assertThat("workDir does not still exist", extendedBuildState.workDir.exists(), is(false));

breakTheProject(appRepo, "master");

StringBuilderWriter badBuildLog = new StringBuilderWriter();
BuildState result2 = runner.build(badBuildLog, "master", null, defaultTimeout, System.getenv()).buildState;
assertThat(buildLog.toString(), result2, is(BuildState.FAILURE));
assertThat(badBuildLog.toString(), containsString("The build could not read 1 project"));
assertThat("workDir does not still exist", extendedBuildState.workDir.exists(), is(false));

}

Expand Down
1 change: 1 addition & 0 deletions src/test/resources/logback-test.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
</appender>

<logger name="org.eclipse.jetty" level="WARN" />
<logger name="com.danielflower.restabuild" level="DEBUG" />

<root level="INFO">
<appender-ref ref="STDOUT" />
Expand Down

0 comments on commit 8c13e90

Please sign in to comment.