Skip to content

Commit 759feaf

Browse files
authored
Fixed various resource leaks (#923)
* Fixed various resource leaks * Review fixes * Fixed race condition in DefoldSdkServiceTest
1 parent 7ab6aa1 commit 759feaf

13 files changed

Lines changed: 134 additions & 94 deletions

File tree

client/src/main/java/com/defold/extender/client/ExtenderClient.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -279,13 +279,15 @@ private void build_async(String platform, String sdkVersion, HttpEntity entity,
279279
response = httpClient.execute(resultRequest);
280280
if (jobStatus == 1) {
281281
log("Job %s completed successfully. Writing result to %s", jobId, destination);
282-
response.getEntity().writeTo(new FileOutputStream(destination));
282+
try(OutputStream os = new FileOutputStream(destination)) {
283+
response.getEntity().writeTo(os);
284+
}
283285
} else {
284286
log("Job %s did not complete successfully. Writing log to %s", jobId, log);
285-
OutputStream os = new FileOutputStream(log);
286-
os.write(String.format("Job id: %s; traceId: %s", jobId, traceId == null ? "null" : traceId).getBytes());
287-
response.getEntity().writeTo(os);
288-
os.close();
287+
try(OutputStream os = new FileOutputStream(log)) {
288+
os.write(String.format("Job id: %s; traceId: %s", jobId, traceId == null ? "null" : traceId).getBytes());
289+
response.getEntity().writeTo(os);
290+
}
289291
throw new ExtenderClientException(String.format("Failed to build source: jobId - %s, traceId - %s", jobId, traceId == null ? "null" : traceId));
290292
}
291293
} else if (statusCode == HttpStatus.SC_NOT_IMPLEMENTED) {
@@ -294,17 +296,17 @@ private void build_async(String platform, String sdkVersion, HttpEntity entity,
294296
String body = responseBody != null ? EntityUtils.toString(responseBody) : "(unknown)";
295297
String error = String.format("%s (trace id - %s)", body, traceId == null ? "null" : traceId);
296298
log(error);
297-
OutputStream os = new FileOutputStream(log);
298-
os.write(error.getBytes());
299-
os.close();
299+
try (OutputStream os = new FileOutputStream(log)) {
300+
os.write(error.getBytes());
301+
}
300302
throw new ExtenderClientException(error);
301303
} else{
302304
String result = String.format("Async build request failed with status code %d %s", statusCode, statusLine.getReasonPhrase());
303305
log(result);
304-
OutputStream os = new FileOutputStream(log);
305-
os.write(result.getBytes());
306-
response.getEntity().writeTo(os);
307-
os.close();
306+
try (OutputStream os = new FileOutputStream(log)) {
307+
os.write(result.getBytes());
308+
response.getEntity().writeTo(os);
309+
}
308310
throw new ExtenderClientException("Failed to build source.");
309311
}
310312
} catch (ExtenderClientException exc) {

client/src/main/java/com/defold/extender/client/ExtenderClientCache.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,8 @@ private static String hash(ExtenderResource extenderResource) throws ExtenderCli
187187
private void saveCache() {
188188
Properties properties = new Properties();
189189
properties.putAll(this.persistentHashes);
190-
try {
191-
properties.store(new FileOutputStream(getCacheFile()), null);
190+
try(OutputStream os = new FileOutputStream(getCacheFile())) {
191+
properties.store(os, null);
192192
} catch (IOException e) {
193193
System.out.println(String.format("Could not store cache to '%s'", getCacheFile().getAbsolutePath()));
194194
}

server/manifestmergetool/src/main/java/com/defold/manifestmergetool/InfoPlistMerger.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,8 @@ public void merge(File main, File[] libraries, File out) throws RuntimeException
249249
}
250250

251251

252-
try {
252+
try(FileWriter writer = new FileWriter(out)) {
253253
FileHandler handler = new FileHandler(basePlist);
254-
FileWriter writer = new FileWriter(out);
255254
handler.save(writer);
256255
} catch (ConfigurationException | IOException e) {
257256
throw new RuntimeException("Failed to write plist: " + e.toString());

server/manifestmergetool/src/main/java/com/defold/manifestmergetool/PrivacyManifestMerger.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,8 @@ public void merge(File main, File[] libraries, File out) throws RuntimeException
162162
}
163163
}
164164

165-
try {
165+
try(ModifyingFileWriter writer = new ModifyingFileWriter(out)) {
166166
FileHandler handler = new FileHandler(basePlist);
167-
ModifyingFileWriter writer = new ModifyingFileWriter(out);
168167
handler.save(writer);
169168
} catch (ConfigurationException | IOException e) {
170169
throw new RuntimeException("Failed to write plist: " + e.toString());

server/src/main/java/com/defold/extender/ExtenderController.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,9 @@ static void receiveUpload(MultipartHttpServletRequest request, File uploadDirect
420420
File sourceCodeArchive = new File(uploadDirectory, ExtenderConst.SOURCE_CODE_ARCHIVE_MAGIC_NAME);
421421
if (sourceCodeArchive.exists()) {
422422
LOGGER.debug("Source code archive found. Unarchiving...");
423-
ZipUtils.unzip(new FileInputStream(sourceCodeArchive), uploadDirectory.toPath());
423+
try (InputStream fis = new FileInputStream(sourceCodeArchive)) {
424+
ZipUtils.unzip(fis, uploadDirectory.toPath());
425+
}
424426
}
425427
}
426428

server/src/main/java/com/defold/extender/services/DefoldSdkService.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.io.IOException;
3333
import java.io.InputStream;
3434
import java.io.InputStreamReader;
35+
import java.io.Reader;
3536
import java.net.HttpURLConnection;
3637
import java.net.URI;
3738
import java.nio.charset.StandardCharsets;
@@ -236,7 +237,9 @@ public CompletableFuture<DefoldSdk> getRemoteSdk(String hash) {
236237
File tmpSdkDirectory = tempDirectoryPath.toFile(); // Either moved or deleted later by Move()
237238

238239
Files.createDirectories(tempDirectoryPath);
239-
ZipUtils.unzip(new FileInputStream(tmpResponseBody), tmpSdkDirectory.toPath());
240+
try (InputStream is = new FileInputStream(tmpResponseBody)) {
241+
ZipUtils.unzip(is, tmpSdkDirectory.toPath());
242+
}
240243

241244
Files.move(tmpSdkDirectory.toPath(), sdkDirectory.toPath(), StandardCopyOption.ATOMIC_MOVE);
242245
isVerified = true;
@@ -350,7 +353,9 @@ private CompletableFuture<JSONObject> downloadSdkMappings(String hash) {
350353
LOGGER.info("Downloading platform sdks mappings from {} ...", url);
351354
InputStream body = response.getBody();
352355
JSONParser parser = new JSONParser();
353-
result = (JSONObject)parser.parse(new InputStreamReader(body));
356+
try (Reader reader = new InputStreamReader(body)) {
357+
result = (JSONObject)parser.parse(reader);
358+
}
354359
break;
355360
}
356361
} catch(IOException|ParseException exc) {

server/src/main/java/com/defold/extender/services/RealGradleService.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import java.io.File;
2525
import java.io.IOException;
26+
import java.io.InputStream;
2627
import java.io.FileInputStream;
2728
import java.nio.charset.StandardCharsets;
2829
import java.nio.file.Files;
@@ -258,7 +259,9 @@ private File resolveDependencyAAR(File dependency, String name, File jobDir) thr
258259

259260
// use job folder as tmp location
260261
File unpackedTmp = new File(jobDir, dependency.getName() + ".tmp");
261-
ZipUtils.unzip(new FileInputStream(dependency), unpackedTmp.toPath());
262+
try (InputStream fis = new FileInputStream(dependency)) {
263+
ZipUtils.unzip(fis, unpackedTmp.toPath());
264+
}
262265
Move(unpackedTmp.toPath(), unpackedTarget.toPath());
263266
return unpackedTarget;
264267
}

server/src/main/java/com/defold/extender/utils/PodBuildUtil.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.io.File;
44
import java.io.FileReader;
55
import java.io.IOException;
6+
import java.io.Reader;
67
import java.nio.charset.StandardCharsets;
78
import java.nio.file.Files;
89
import java.nio.file.StandardOpenOption;
@@ -91,9 +92,9 @@ public static File generateVFSOverlay(PodBuildSpec spec, Map<String, Collection<
9192

9293
public static File mergeVFSOverlays(File overlayA, File overlayB) {
9394
JSONParser parser = new JSONParser();
94-
try {
95-
JSONObject parsedOverlayA = (JSONObject)parser.parse(new FileReader(overlayA));
96-
JSONObject parsedOverlayB = (JSONObject)parser.parse(new FileReader(overlayB));
95+
try(Reader readerA = new FileReader(overlayA); Reader readerB = new FileReader(overlayB)) {
96+
JSONObject parsedOverlayA = (JSONObject)parser.parse(readerA);
97+
JSONObject parsedOverlayB = (JSONObject)parser.parse(readerB);
9798

9899
JSONArray roots = (JSONArray)parsedOverlayA.get("roots");
99100
roots.addAll((JSONArray)parsedOverlayB.get("roots"));

server/src/test/java/com/defold/extender/ExtenderUtilTest.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.io.FileInputStream;
1717
import java.io.FileNotFoundException;
1818
import java.io.IOException;
19+
import java.io.InputStream;
1920
import java.nio.charset.StandardCharsets;
2021
import java.nio.file.Files;
2122
import java.nio.file.Path;
@@ -292,11 +293,16 @@ public void testCreatePlatformConfig() throws ExtenderException {
292293

293294
@Test
294295
public void testSHA256Checksum() throws NoSuchAlgorithmException, FileNotFoundException, IOException {
295-
String calculatedSha = ExtenderUtil.calculateSHA256(new FileInputStream("test-data/checksum_sdk/test_sdk.zip"));
296-
String expectedSha = new String(Files.readAllBytes(Path.of("test-data/checksum_sdk/test_sdk.sha256")), StandardCharsets.UTF_8);
297-
assertEquals(expectedSha, calculatedSha);
298-
299-
assertThrows(FileNotFoundException.class, () -> ExtenderUtil.calculateSHA256(new FileInputStream("test-data/checksum_sdk/non-exist.zip")));
296+
try (InputStream is = new FileInputStream("test-data/checksum_sdk/test_sdk.zip")) {
297+
String calculatedSha = ExtenderUtil.calculateSHA256(is);
298+
String expectedSha = new String(Files.readAllBytes(Path.of("test-data/checksum_sdk/test_sdk.sha256")), StandardCharsets.UTF_8);
299+
assertEquals(expectedSha, calculatedSha);
300+
}
301+
assertThrows(FileNotFoundException.class, () -> {
302+
try(InputStream is = new FileInputStream("test-data/checksum_sdk/non-exist.zip")) {
303+
ExtenderUtil.calculateSHA256(is);
304+
}
305+
});
300306
}
301307

302308
@Test

server/src/test/java/com/defold/extender/ZipUtilsTest.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
import java.io.FileInputStream;
99
import java.io.FileOutputStream;
1010
import java.io.IOException;
11+
import java.io.InputStream;
12+
import java.io.OutputStream;
1113
import java.nio.file.Files;
1214
import java.nio.file.Path;
1315
import java.util.ArrayList;
@@ -28,9 +30,13 @@ public void zipAndUnzipFiles() throws IOException {
2830
files.add(sourceFile1.toFile());
2931
files.add(sourceFile2.toFile());
3032

31-
ZipUtils.zip(new FileOutputStream(destinationFile.toFile()), null, files);
33+
try (OutputStream os = new FileOutputStream(destinationFile.toFile())) {
34+
ZipUtils.zip(os, null, files);
35+
}
3236

33-
ZipUtils.unzip(new FileInputStream(destinationFile.toFile()), targetDirectory);
37+
try(InputStream is = new FileInputStream(destinationFile.toFile())) {
38+
ZipUtils.unzip(is, targetDirectory);
39+
}
3440

3541
assertEquals(2, targetDirectory.toFile().listFiles().length);
3642
}

0 commit comments

Comments
 (0)