Skip to content

Commit

Permalink
Fixed issue in isort integration (it could deadlock as it could write…
Browse files Browse the repository at this point in the history
… back while the input is still being written but there was no one reading it).'
  • Loading branch information
fabioz committed Jul 24, 2023
1 parent 099e8b8 commit 1c5dac8
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,8 @@ public static String formatWithBlack(String filepath, IPythonNature nature, IDoc
if (pythonFileEncoding == null) {
pythonFileEncoding = "utf-8";
}
boolean failedWrite = false;
try {
process.getOutputStream().write(doc.get().getBytes(pythonFileEncoding));
} catch (Exception e) {
failedWrite = true;
}
Tuple<String, String> processOutput = ProcessUtils.getProcessOutput(process, cmdarrayAsStr,
new NullProgressMonitor(), pythonFileEncoding);

if (process.exitValue() != 0 || failedWrite) {
Log.log("Black formatter exited with: " + process.exitValue() + " failedWrite: " + failedWrite
+ "\nStdout:\n" + processOutput.o1
+ "\nStderr:\n" + processOutput.o2);
return null;
}
return processOutput.o1;
return RunnerCommon.writeContentsAndGetOutput(doc.get().getBytes(pythonFileEncoding), pythonFileEncoding,
process, cmdarrayAsStr, "black");
} catch (Exception e) {
Log.log(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.python.pydev.core.log.Log;
import org.python.pydev.plugin.nature.SystemPythonNature;
import org.python.pydev.shared_core.io.FileUtils;
import org.python.pydev.shared_core.process.ProcessUtils;
import org.python.pydev.shared_core.structure.Tuple;
import org.python.pydev.shared_core.utils.ArrayUtils;

Expand Down Expand Up @@ -61,28 +60,13 @@ public static String formatWithISort(File targetFile, IPythonNature nature, Stri
final String[] args = ArrayUtils.concatArrays(pathArgs, isortArguments, known.toArray(new String[0]),
new String[] { "-d", "-" });
Tuple<Process, String> processInfo = pythonRunner.createProcessFromModuleName("isort",
args,
workingDir, new NullProgressMonitor());
args, workingDir, new NullProgressMonitor());
process = processInfo.o1;
cmdarrayAsStr = processInfo.o2;
}

boolean failedWrite = false;
try {
process.getOutputStream().write(fileContents.getBytes(encoding));
} catch (Exception e) {
failedWrite = true;
}
Tuple<String, String> processOutput = ProcessUtils.getProcessOutput(process, cmdarrayAsStr,
new NullProgressMonitor(), encoding);

if (process.exitValue() != 0 || failedWrite) {
Log.log("isort exited with: " + process.exitValue() + " failedWrite: " + failedWrite
+ "\nStdout:\n" + processOutput.o1
+ "\nStderr:\n" + processOutput.o2);
return null;
}
return processOutput.o1;
return RunnerCommon.writeContentsAndGetOutput(fileContents.getBytes(encoding), encoding, process, cmdarrayAsStr,
"isort");
} catch (Exception e) {
Log.log(e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.python.pydev.core.pep8;

import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import org.eclipse.core.runtime.NullProgressMonitor;
import org.python.pydev.core.log.Log;
import org.python.pydev.shared_core.process.ProcessUtils;
import org.python.pydev.shared_core.structure.Tuple;

public class RunnerCommon {

public static String writeContentsAndGetOutput(byte[] fileContents, String encoding, Process process,
String cmdarrayAsStr, String toolName) throws InterruptedException, ExecutionException, TimeoutException {
CompletableFuture<Tuple<String, String>> future = new CompletableFuture<>();

// Isort will start to write as we're writing the input, so, we need to start
// reading before we write (it can't be sequential otherwise it can deadlock).

Thread t = new Thread() {
@Override
public void run() {
try {
boolean closeOutput = false;
future.complete(ProcessUtils.getProcessOutput(process, cmdarrayAsStr,
new NullProgressMonitor(), encoding, closeOutput));
} catch (Throwable e) {
future.completeExceptionally(e);
}
}
};
t.start();

boolean failedWrite = false;
try {
process.getOutputStream().write(fileContents);
process.getOutputStream().flush();
process.getOutputStream().close();
} catch (Exception e) {
failedWrite = true;
}

// Wait a full minute before bailing out.
try {
Tuple<String, String> processOutput = future.get(60, TimeUnit.SECONDS);

if (process.exitValue() != 0 || failedWrite) {
Log.log(toolName + " exited with: " + process.exitValue() + " failedWrite: " + failedWrite
+ "\nStdout:\n" + processOutput.o1
+ "\nStderr:\n" + processOutput.o2);
return null;
}
return processOutput.o1;
} catch (Exception e) {
Log.log(e);
}
return null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,30 @@
import org.python.pydev.shared_core.utils.PlatformUtils;

public class ProcessUtils {
public static Tuple<String, String> getProcessOutput(Process process, String executionString,
IProgressMonitor monitor, String encoding) {
boolean closeOutput = true;
return getProcessOutput(process, executionString, monitor, encoding, closeOutput);
}

/**
* @param process process from where the output should be gotten
* @param executionString string to execute (only for errors)
* @param monitor monitor for giving progress
* @return a tuple with the output of stdout and stderr
*/
public static Tuple<String, String> getProcessOutput(Process process, String executionString,
IProgressMonitor monitor, String encoding) {
IProgressMonitor monitor, String encoding, boolean closeOutput) {
if (monitor == null) {
monitor = new NullProgressMonitor();
}
if (process != null) {

try {
process.getOutputStream().close(); //we won't write to it...
} catch (IOException e2) {
if (closeOutput) {
try {
process.getOutputStream().close(); //we won't write to it...
} catch (IOException e2) {
}
}

monitor.setTaskName("Reading output...");
Expand Down

0 comments on commit 1c5dac8

Please sign in to comment.