Skip to content

Commit 7d99a38

Browse files
committed
fix: avoid potential hang as it blocks on process.getInputStream().readAllBytes() before timeout checking
1 parent 025fdf2 commit 7d99a38

1 file changed

Lines changed: 51 additions & 21 deletions

File tree

src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@
4545
import java.util.HashSet;
4646
import java.util.Map;
4747
import java.util.Set;
48+
import java.util.concurrent.CompletableFuture;
49+
import java.util.concurrent.ExecutionException;
4850
import java.util.concurrent.TimeUnit;
51+
import java.util.concurrent.TimeoutException;
4952
import java.util.logging.Logger;
5053
import org.tomlj.Toml;
5154
import org.tomlj.TomlParseResult;
@@ -341,23 +344,53 @@ private CargoMetadata executeCargoMetadata() throws IOException, InterruptedExce
341344
.directory(workingDir.toFile())
342345
.start();
343346

344-
String output;
345-
try (InputStream is = process.getInputStream()) {
346-
output = new String(is.readAllBytes(), StandardCharsets.UTF_8);
347-
}
347+
// Use CompletableFuture to read output streams concurrently to avoid two potential deadlocks:
348+
// 1. buffer deadlock (process blocks on write when output buffers fill up while Java waits for
349+
// process completion)
350+
// 2. stalled process deadlock (readAllBytes() hangs forever when cargo process stalls
351+
// completely
352+
// with no timeout protection)
353+
CompletableFuture<String> outputFuture =
354+
CompletableFuture.supplyAsync(
355+
() -> {
356+
try (InputStream is = process.getInputStream()) {
357+
return new String(is.readAllBytes(), StandardCharsets.UTF_8);
358+
} catch (IOException e) {
359+
return "";
360+
}
361+
});
362+
363+
CompletableFuture<String> errorFuture =
364+
CompletableFuture.supplyAsync(
365+
() -> {
366+
try (InputStream is = process.getErrorStream()) {
367+
return new String(is.readAllBytes(), StandardCharsets.UTF_8);
368+
} catch (IOException e) {
369+
return "";
370+
}
371+
});
348372

349373
boolean finished = process.waitFor(TIMEOUT, TimeUnit.SECONDS);
350374

375+
if (!finished) {
376+
process.destroyForcibly();
377+
throw new InterruptedException("cargo metadata timed out after " + TIMEOUT + " seconds");
378+
}
379+
380+
String output;
381+
String errorOutput;
382+
try {
383+
output = outputFuture.get(TIMEOUT, TimeUnit.SECONDS);
384+
errorOutput = errorFuture.get(TIMEOUT, TimeUnit.SECONDS);
385+
} catch (ExecutionException | TimeoutException e) {
386+
log.warning("Failed to read process output: " + e.getMessage());
387+
return null;
388+
}
389+
390+
// Safe to call exitValue() - we confirmed the process finished
351391
int exitCode = process.exitValue();
352392

353393
if (exitCode != 0) {
354-
String errorOutput = "";
355-
try (InputStream errorStream = process.getErrorStream()) {
356-
errorOutput = new String(errorStream.readAllBytes(), StandardCharsets.UTF_8);
357-
} catch (IOException e) {
358-
log.warning("Failed to read error stream: " + e.getMessage());
359-
}
360-
361394
String errorMessage = "cargo metadata failed with exit code: " + exitCode;
362395
if (!errorOutput.isEmpty()) {
363396
errorMessage += ". Error: " + errorOutput.trim();
@@ -373,11 +406,6 @@ private CargoMetadata executeCargoMetadata() throws IOException, InterruptedExce
373406
return null;
374407
}
375408

376-
if (!finished) {
377-
process.destroyForcibly();
378-
throw new InterruptedException("cargo metadata timed out after " + TIMEOUT + " seconds");
379-
}
380-
381409
try {
382410
CargoMetadata metadata = MAPPER.readValue(output, CargoMetadata.class);
383411
if (debugLoggingIsNeeded()) {
@@ -552,12 +580,14 @@ private DependencyInfo getPackageInfo(String packageId, Map<String, CargoPackage
552580
public CargoProvider(Path manifest) {
553581
super(Type.CARGO, manifest);
554582
this.cargoExecutable = Operations.getExecutable("cargo", "--version");
555-
if (cargoExecutable != null) {
556-
log.info("Found cargo executable: " + cargoExecutable);
557-
} else {
558-
log.warning("Cargo executable not found - dependency analysis will not work");
583+
if (debugLoggingIsNeeded()) {
584+
if (cargoExecutable != null) {
585+
log.info("Found cargo executable: " + cargoExecutable);
586+
} else {
587+
log.warning("Cargo executable not found - dependency analysis will not work");
588+
}
589+
log.info("Initialized RustProvider for manifest: " + manifest);
559590
}
560-
log.info("Initialized RustProvider for manifest: " + manifest);
561591
}
562592

563593
@Override

0 commit comments

Comments
 (0)