Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
8236282: [macos] Find permanent solution to macOS test timeout proble…
…m JDK-8235738

Reviewed-by: herrick, asemenyuk
  • Loading branch information
Alexander Matveev committed Jun 9, 2020
1 parent 71d646a commit 976c469305752b55918ba360e1b308a6606cb1f4
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -25,8 +25,10 @@
package jdk.incubator.jpackage.internal;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.Supplier;
@@ -48,8 +50,8 @@ Executor saveOutput(boolean v) {
return this;
}

Executor setWaitBeforeOutput(boolean v) {
waitBeforeOutput = v;
Executor setWriteOutputToFile(boolean v) {
writeOutputToFile = v;
return this;
}

@@ -80,8 +82,13 @@ int execute() throws IOException {
output = null;

boolean needProcessOutput = outputConsumer != null || Log.isVerbose() || saveOutput;
File outputFile = null;
if (needProcessOutput) {
pb.redirectErrorStream(true);
if (writeOutputToFile) {
outputFile = File.createTempFile("jpackageOutputTempFile", ".tmp");
pb.redirectOutput(outputFile);
}
} else {
// We are not going to read process output, so need to notify
// ProcessBuilder about this. Otherwise some processes might just
@@ -94,7 +101,7 @@ int execute() throws IOException {
Process p = pb.start();

int code = 0;
if (waitBeforeOutput) {
if (writeOutputToFile) {
try {
code = p.waitFor();
} catch (InterruptedException ex) {
@@ -104,25 +111,17 @@ int execute() throws IOException {
}

if (needProcessOutput) {
try (var br = new BufferedReader(new InputStreamReader(
p.getInputStream()))) {
final List<String> savedOutput;
// Need to save output if explicitely requested (saveOutput=true) or
// if will be used used by multiple consumers
if ((outputConsumer != null && Log.isVerbose()) || saveOutput) {
savedOutput = br.lines().collect(Collectors.toList());
if (saveOutput) {
output = savedOutput;
}
} else {
savedOutput = null;
}
final List<String> savedOutput;
Supplier<Stream<String>> outputStream;

Supplier<Stream<String>> outputStream = () -> {
if (writeOutputToFile) {
savedOutput = Files.readAllLines(outputFile.toPath());
outputFile.delete();
outputStream = () -> {
if (savedOutput != null) {
return savedOutput.stream();
}
return br.lines();
return null;
};

if (Log.isVerbose()) {
@@ -132,21 +131,50 @@ int execute() throws IOException {
if (outputConsumer != null) {
outputConsumer.accept(outputStream.get());
}
} else {
try (var br = new BufferedReader(new InputStreamReader(
p.getInputStream()))) {
// Need to save output if explicitely requested (saveOutput=true) or
// if will be used used by multiple consumers
if ((outputConsumer != null && Log.isVerbose()) || saveOutput) {
savedOutput = br.lines().collect(Collectors.toList());
if (saveOutput) {
output = savedOutput;
}
} else {
savedOutput = null;
}

outputStream = () -> {
if (savedOutput != null) {
return savedOutput.stream();
}
return br.lines();
};

if (savedOutput == null) {
// For some processes on Linux if the output stream
// of the process is opened but not consumed, the process
// would exit with code 141.
// It turned out that reading just a single line of process
// output fixes the problem, but let's process
// all of the output, just in case.
br.lines().forEach(x -> {});
if (Log.isVerbose()) {
outputStream.get().forEach(Log::verbose);
}

if (outputConsumer != null) {
outputConsumer.accept(outputStream.get());
}

if (savedOutput == null) {
// For some processes on Linux if the output stream
// of the process is opened but not consumed, the process
// would exit with code 141.
// It turned out that reading just a single line of process
// output fixes the problem, but let's process
// all of the output, just in case.
br.lines().forEach(x -> {});
}
}
}
}

try {
if (!waitBeforeOutput) {
if (!writeOutputToFile) {
code = p.waitFor();
}
return code;
@@ -175,7 +203,7 @@ private static String createLogMessage(ProcessBuilder pb) {

private ProcessBuilder pb;
private boolean saveOutput;
private boolean waitBeforeOutput;
private boolean writeOutputToFile;
private List<String> output;
private Consumer<Stream<String>> outputConsumer;
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -29,7 +29,6 @@
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.nio.channels.FileChannel;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -136,12 +135,14 @@ public static void exec(ProcessBuilder pb)
exec(pb, false, null, false);
}

// Reading output from some processes (currently known "hdiutil attach" might hang even if process already
// exited. Only possible workaround found in "hdiutil attach" case is to wait for process to exit before
// reading output.
public static void exec(ProcessBuilder pb, boolean waitBeforeOutput)
// See JDK-8236282
// Reading output from some processes (currently known "hdiutil attach")
// might hang even if process already exited. Only possible workaround found
// in "hdiutil attach" case is to redirect the output to a temp file and then
// read this file back.
public static void exec(ProcessBuilder pb, boolean writeOutputToFile)
throws IOException {
exec(pb, false, null, waitBeforeOutput);
exec(pb, false, null, writeOutputToFile);
}

static void exec(ProcessBuilder pb, boolean testForPresenceOnly,
@@ -150,9 +151,10 @@ static void exec(ProcessBuilder pb, boolean testForPresenceOnly,
}

static void exec(ProcessBuilder pb, boolean testForPresenceOnly,
PrintStream consumer, boolean waitBeforeOutput) throws IOException {
PrintStream consumer, boolean writeOutputToFile) throws IOException {
List<String> output = new ArrayList<>();
Executor exec = Executor.of(pb).setWaitBeforeOutput(waitBeforeOutput).setOutputConsumer(lines -> {
Executor exec = Executor.of(pb).setWriteOutputToFile(writeOutputToFile)
.setOutputConsumer(lines -> {
lines.forEach(output::add);
if (consumer != null) {
output.forEach(consumer::println);

0 comments on commit 976c469

Please sign in to comment.