Skip to content

Commit 4630494

Browse files
committed
fix: resolve resource leaks in GHService.getDiffStats()
Extract JGit operations to DiffStatsCalculator with proper try-with-resources to ensure ObjectReader, DiffFormatter, and RevWalk instances are always closed. Fixes: - ObjectReader leaked in dryRun code path - DiffFormatter never closed in any path - RevWalk instances never closed in committed path Fixes #1660
1 parent 7eb6a61 commit 4630494

1 file changed

Lines changed: 145 additions & 0 deletions

File tree

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
package io.jenkins.tools.pluginmodernizer.core.github;
2+
3+
import io.jenkins.tools.pluginmodernizer.core.config.Settings;
4+
import io.jenkins.tools.pluginmodernizer.core.model.DiffStats;
5+
import io.jenkins.tools.pluginmodernizer.core.model.Plugin;
6+
import java.io.ByteArrayOutputStream;
7+
import java.io.File;
8+
import java.io.IOException;
9+
import java.nio.file.Path;
10+
import java.util.List;
11+
import org.eclipse.jgit.api.Git;
12+
import org.eclipse.jgit.api.errors.GitAPIException;
13+
import org.eclipse.jgit.diff.DiffEntry;
14+
import org.eclipse.jgit.diff.DiffFormatter;
15+
import org.eclipse.jgit.diff.Edit;
16+
import org.eclipse.jgit.diff.EditList;
17+
import org.eclipse.jgit.diff.RawTextComparator;
18+
import org.eclipse.jgit.dircache.DirCacheIterator;
19+
import org.eclipse.jgit.lib.ObjectId;
20+
import org.eclipse.jgit.lib.ObjectReader;
21+
import org.eclipse.jgit.lib.Repository;
22+
import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
23+
import org.eclipse.jgit.revwalk.RevWalk;
24+
import org.eclipse.jgit.treewalk.CanonicalTreeParser;
25+
import org.eclipse.jgit.treewalk.FileTreeIterator;
26+
import org.eclipse.jgit.treewalk.WorkingTreeIterator;
27+
import org.slf4j.Logger;
28+
import org.slf4j.LoggerFactory;
29+
30+
public class DiffStatsCalculator {
31+
32+
private static final Logger LOG = LoggerFactory.getLogger(DiffStatsCalculator.class);
33+
34+
public DiffStats calculate(Plugin plugin, boolean dryRun) throws IOException, GitAPIException {
35+
Path gitDirPath = Settings.DEFAULT_CACHE_PATH
36+
.resolve(plugin.getName())
37+
.resolve("sources")
38+
.resolve(".git")
39+
.normalize();
40+
File gitDir = gitDirPath.toFile();
41+
42+
try (Repository repository = new FileRepositoryBuilder()
43+
.setGitDir(gitDir)
44+
.readEnvironment()
45+
.findGitDir()
46+
.build();
47+
Git git = new Git(repository)) {
48+
49+
return calculateDiff(repository, git, plugin, dryRun);
50+
}
51+
}
52+
53+
private DiffStats calculateDiff(Repository repository, Git git, Plugin plugin, boolean dryRun)
54+
throws IOException, GitAPIException {
55+
int additions = 0;
56+
int deletions = 0;
57+
int changedFiles = 0;
58+
59+
try (ObjectReader reader = repository.newObjectReader();
60+
DiffFormatter formatter = new DiffFormatter(new ByteArrayOutputStream())) {
61+
formatter.setRepository(repository);
62+
formatter.setDiffComparator(RawTextComparator.DEFAULT);
63+
formatter.setDetectRenames(true);
64+
65+
if (dryRun) {
66+
return calculateUnstagedDiff(repository, git, reader, formatter);
67+
} else {
68+
return calculateCommittedDiff(repository, git, plugin, reader, formatter);
69+
}
70+
}
71+
}
72+
73+
private DiffStats calculateUnstagedDiff(
74+
Repository repository, Git git, ObjectReader reader, DiffFormatter formatter)
75+
throws IOException, GitAPIException {
76+
int additions = 0;
77+
int deletions = 0;
78+
int changedFiles = 0;
79+
80+
DirCacheIterator indexTree = new DirCacheIterator(repository.readDirCache());
81+
FileTreeIterator workingTree = new FileTreeIterator(repository);
82+
83+
List<DiffEntry> unstagedDiffs = git.diff()
84+
.setOldTree(indexTree)
85+
.setNewTree(workingTree)
86+
.setShowNameAndStatusOnly(false)
87+
.call();
88+
89+
for (DiffEntry diff : unstagedDiffs) {
90+
try {
91+
EditList edits = formatter.toFileHeader(diff).toEditList();
92+
for (Edit edit : edits) {
93+
additions += edit.getEndB() - edit.getBeginB();
94+
deletions += edit.getEndA() - edit.getBeginA();
95+
}
96+
changedFiles++;
97+
} catch (org.eclipse.jgit.errors.MissingObjectException e) {
98+
LOG.warn("Skipping diff for {}: {}", diff.getNewPath(), e.getMessage());
99+
}
100+
}
101+
102+
return new DiffStats(additions, deletions, changedFiles);
103+
}
104+
105+
private DiffStats calculateCommittedDiff(
106+
Repository repository, Git git, Plugin plugin, ObjectReader reader, DiffFormatter formatter)
107+
throws IOException, GitAPIException {
108+
int additions = 0;
109+
int deletions = 0;
110+
int changedFiles = 0;
111+
112+
ObjectId head = repository.resolve("HEAD");
113+
String defaultBranchName = plugin.getRemoteRepository(null).getDefaultBranch();
114+
ObjectId defaultBranch = repository.resolve("refs/heads/" + defaultBranchName);
115+
116+
if (defaultBranch == null) {
117+
throw new IOException("Could not resolve default branch.");
118+
}
119+
120+
try (RevWalk oldWalk = new RevWalk(repository);
121+
RevWalk newWalk = new RevWalk(repository)) {
122+
CanonicalTreeParser oldTree = new CanonicalTreeParser();
123+
CanonicalTreeParser newTree = new CanonicalTreeParser();
124+
oldTree.reset(reader, oldWalk.parseTree(defaultBranch));
125+
newTree.reset(reader, newWalk.parseTree(head));
126+
127+
List<DiffEntry> committedDiffs = git.diff()
128+
.setOldTree(oldTree)
129+
.setNewTree(newTree)
130+
.setShowNameAndStatusOnly(false)
131+
.call();
132+
133+
for (DiffEntry diff : committedDiffs) {
134+
EditList edits = formatter.toFileHeader(diff).toEditList();
135+
for (Edit edit : edits) {
136+
additions += edit.getEndB() - edit.getBeginB();
137+
deletions += edit.getEndA() - edit.getBeginA();
138+
}
139+
changedFiles++;
140+
}
141+
}
142+
143+
return new DiffStats(additions, deletions, changedFiles);
144+
}
145+
}

0 commit comments

Comments
 (0)