Skip to content

Commit

Permalink
Avoid deadlock when running Elm make via dedicated OS process
Browse files Browse the repository at this point in the history
  • Loading branch information
Viir committed Dec 20, 2024
1 parent e61ea0e commit a70ffd7
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 85 deletions.
84 changes: 5 additions & 79 deletions implement/pine/Elm/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
using Pine.PineVM;
using Pine.Elm019;
using System.Collections.Immutable;
using System.Text;
using System.Threading.Tasks;

namespace Pine.Elm;

Expand Down Expand Up @@ -1026,9 +1028,10 @@ public void TextDocument_didSave(
Log("Begin elm make for " + textDocumentUri);

var elmMakeOutput =
ElmMake(
ElmMakeRunner.ElmMakeAsync(
workingDirectoryAbsolute: workingDirectoryAbsolute,
pathToFileWithElmEntryPoint: localPath);
pathToFileWithElmEntryPoint: localPath)
.Result;

Log("Completed elm make for " + textDocumentUri + " in " +
CommandLineInterface.FormatIntegerForDisplay(clock.ElapsedMilliseconds) + " ms");
Expand Down Expand Up @@ -1153,83 +1156,6 @@ public static Result<string, ElmMakeReport> ParseReportFromElmMake(string elmMak
}
}

/// <summary>
/// Use the 'elm make' command on the Elm executable file.
/// </summary>
public static ExecutableFile.ProcessOutput ElmMake(
string workingDirectoryAbsolute,
string pathToFileWithElmEntryPoint)
{
var arguments =
string.Join(" ", ["make", pathToFileWithElmEntryPoint, "--report=json --output=/dev/null"]);

var executableFile =
BlobLibrary.LoadFileForCurrentOs(ElmTime.Elm019.Elm019Binaries.ElmExecutableFileByOs)
??
throw new System.Exception("Failed to load elm-format executable file");

if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
System.IO.File.SetUnixFileMode(
executableFile.cacheFilePath,
ExecutableFile.UnixFileModeForExecutableFile);
}

var process = new System.Diagnostics.Process
{
StartInfo = new System.Diagnostics.ProcessStartInfo
{
WorkingDirectory = workingDirectoryAbsolute,
FileName = executableFile.cacheFilePath,
Arguments = arguments,
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true,
CreateNoWindow = true,
},
};


// Avoid elm make failing on `getAppUserDataDirectory`.
/* Also, work around problems with elm make like this:
-- HTTP PROBLEM ----------------------------------------------------------------
The following HTTP request failed:
<https://github.com/elm/core/zipball/1.0.0/>
Here is the error message I was able to extract:
HttpExceptionRequest Request { host = "github.com" port = 443 secure = True
requestHeaders = [("User-Agent","elm/0.19.0"),("Accept-Encoding","gzip")]
path = "/elm/core/zipball/1.0.0/" queryString = "" method = "GET" proxy =
Nothing rawBody = False redirectCount = 10 responseTimeout =
ResponseTimeoutDefault requestVersion = HTTP/1.1 } (StatusCodeException
(Response {responseStatus = Status {statusCode = 429, statusMessage = "Too
Many Requests"}, responseVersion = HTTP/1.1, responseHeaders =
[("Server","GitHub.com"),("Date","Sun, 18 Nov 2018 16:53:18
GMT"),("Content-Type","text/html"),("Transfer-Encoding","chunked"),("Status","429
Too Many
Requests"),("Retry-After","120")
To avoid elm make failing with this error, break isolation here and reuse elm home directory.
An alternative would be retrying when this error is parsed from `commandResults.processOutput.StandardError`.
*/
process.StartInfo.Environment["ELM_HOME"] = ElmTime.Elm019.Elm019Binaries.GetElmHomeDirectory();

process.Start();
var standardOutput = process.StandardOutput.ReadToEnd();
var standardError = process.StandardError.ReadToEnd();

process.WaitForExit();
var exitCode = process.ExitCode;
process.Close();

return new ExecutableFile.ProcessOutput(
StandardError: standardError,
StandardOutput: standardOutput,
ExitCode: exitCode);
}

public static string? FindElmJsonFile(string elmModuleFilePath)
{
var directoryName = System.IO.Path.GetDirectoryName(elmModuleFilePath);
Expand Down
128 changes: 128 additions & 0 deletions implement/pine/Elm019/ElmMakeRunner.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
using System.Runtime.InteropServices;
using System.Text;
using System.Threading.Tasks;

namespace Pine.Elm019;

public static class ElmMakeRunner
{
public static async Task<ExecutableFile.ProcessOutput> ElmMakeAsync(
string workingDirectoryAbsolute,
string pathToFileWithElmEntryPoint)
{
var arguments =
string.Join(" ", ["make", pathToFileWithElmEntryPoint, "--report=json --output=/dev/null"]);

var executableFile =
BlobLibrary.LoadFileForCurrentOs(ElmTime.Elm019.Elm019Binaries.ElmExecutableFileByOs)
??
throw new System.Exception("Failed to load elm-format executable file");

if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
System.IO.File.SetUnixFileMode(
executableFile.cacheFilePath,
ExecutableFile.UnixFileModeForExecutableFile);
}

var process = new System.Diagnostics.Process
{
StartInfo = new System.Diagnostics.ProcessStartInfo
{
WorkingDirectory = workingDirectoryAbsolute,
FileName = executableFile.cacheFilePath,
Arguments = arguments,
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true,
CreateNoWindow = true,
},
};


// Avoid elm make failing on `getAppUserDataDirectory`.
/* Also, work around problems with elm make like this:
-- HTTP PROBLEM ----------------------------------------------------------------
The following HTTP request failed:
<https://github.com/elm/core/zipball/1.0.0/>
Here is the error message I was able to extract:
HttpExceptionRequest Request { host = "github.com" port = 443 secure = True
requestHeaders = [("User-Agent","elm/0.19.0"),("Accept-Encoding","gzip")]
path = "/elm/core/zipball/1.0.0/" queryString = "" method = "GET" proxy =
Nothing rawBody = False redirectCount = 10 responseTimeout =
ResponseTimeoutDefault requestVersion = HTTP/1.1 } (StatusCodeException
(Response {responseStatus = Status {statusCode = 429, statusMessage = "Too
Many Requests"}, responseVersion = HTTP/1.1, responseHeaders =
[("Server","GitHub.com"),("Date","Sun, 18 Nov 2018 16:53:18
GMT"),("Content-Type","text/html"),("Transfer-Encoding","chunked"),("Status","429
Too Many
Requests"),("Retry-After","120")
To avoid elm make failing with this error, break isolation here and reuse elm home directory.
An alternative would be retrying when this error is parsed from `commandResults.processOutput.StandardError`.
*/
process.StartInfo.Environment["ELM_HOME"] = ElmTime.Elm019.Elm019Binaries.GetElmHomeDirectory();

var standardOutputBuilder = new StringBuilder();
var standardErrorBuilder = new StringBuilder();

// We'll use TaskCompletionSource to wait until all output has been read
var stdoutTcs = new TaskCompletionSource<bool>();
var stderrTcs = new TaskCompletionSource<bool>();

// Event handler for standard output
process.OutputDataReceived += (sender, e) =>
{
if (e.Data is null)
{
// No more output
stdoutTcs.TrySetResult(true);
}
else
{
standardOutputBuilder.AppendLine(e.Data);
}
};

// Event handler for standard error
process.ErrorDataReceived += (sender, e) =>
{
if (e.Data is null)
{
// No more error output
stderrTcs.TrySetResult(true);
}
else
{
standardErrorBuilder.AppendLine(e.Data);
}
};

// Start the process and begin asynchronous reads
if (!process.Start())
{
throw new System.Exception("Failed to start elm make process.");
}

process.BeginOutputReadLine();
process.BeginErrorReadLine();

// Wait for the process to exit, then for all output to be collected
await process.WaitForExitAsync();

// At this point, the process has exited, but we need to ensure we collected all output lines.
// The event handlers complete when they receive a null Data line.
await Task.WhenAll(stdoutTcs.Task, stderrTcs.Task);

var exitCode = process.ExitCode;
process.Close();

return new ExecutableFile.ProcessOutput(
StandardError: standardErrorBuilder.ToString(),
StandardOutput: standardOutputBuilder.ToString(),
ExitCode: exitCode);
}
}
79 changes: 73 additions & 6 deletions implement/pine/Pine/ExecutableFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading.Tasks;

namespace Pine;

Expand All @@ -16,7 +18,25 @@ public record ProcessOutput(
string StandardOutput,
int ExitCode);


public static (ProcessOutput processOutput, IReadOnlyCollection<(IReadOnlyList<string> path, ReadOnlyMemory<byte> content)> resultingFiles) ExecuteFileWithArguments(
IReadOnlyDictionary<IReadOnlyList<string>, ReadOnlyMemory<byte>> environmentFilesNotExecutable,
ReadOnlyMemory<byte> executableFile,
string arguments,
IDictionary<string, string>? environmentStrings,
IReadOnlyList<string>? workingDirectoryRelative = null,
IReadOnlyDictionary<IReadOnlyList<string>, ReadOnlyMemory<byte>>? environmentFilesExecutable = null,
IReadOnlyDictionary<string, ReadOnlyMemory<byte>>? environmentPathExecutableFiles = null) =>
ExecuteFileWithArgumentsAsync(
environmentFilesNotExecutable,
executableFile,
arguments,
environmentStrings,
workingDirectoryRelative,
environmentFilesExecutable,
environmentPathExecutableFiles).Result;

public static async Task<(ProcessOutput processOutput, IReadOnlyCollection<(IReadOnlyList<string> path, ReadOnlyMemory<byte> content)> resultingFiles)> ExecuteFileWithArgumentsAsync(
IReadOnlyDictionary<IReadOnlyList<string>, ReadOnlyMemory<byte>> environmentFilesNotExecutable,
ReadOnlyMemory<byte> executableFile,
string arguments,
Expand Down Expand Up @@ -130,12 +150,59 @@ void writeEnvironmentFile(
foreach (var envString in environmentStringsWithExecutableFiles.EmptyIfNull())
process.StartInfo.Environment[envString.Key] = envString.Value;

process.Start();
var standardOutput = process.StandardOutput.ReadToEnd();
var standardError = process.StandardError.ReadToEnd();
var standardOutputBuilder = new StringBuilder();
var standardErrorBuilder = new StringBuilder();

// We'll use TaskCompletionSource to wait until all output has been read
var stdoutTcs = new TaskCompletionSource<bool>();
var stderrTcs = new TaskCompletionSource<bool>();

// Event handler for standard output
process.OutputDataReceived += (sender, e) =>
{
if (e.Data is null)
{
// No more output
stdoutTcs.TrySetResult(true);
}
else
{
standardOutputBuilder.AppendLine(e.Data);
}
};

// Event handler for standard error
process.ErrorDataReceived += (sender, e) =>
{
if (e.Data is null)
{
// No more error output
stderrTcs.TrySetResult(true);
}
else
{
standardErrorBuilder.AppendLine(e.Data);
}
};

// Start the process and begin asynchronous reads
if (!process.Start())
{
throw new Exception("Failed to start elm make process.");
}

process.BeginOutputReadLine();
process.BeginErrorReadLine();

// Wait for the process to exit, then for all output to be collected
await process.WaitForExitAsync();

// At this point, the process has exited, but we need to ensure we collected all output lines.
// The event handlers complete when they receive a null Data line.
await Task.WhenAll(stdoutTcs.Task, stderrTcs.Task);

process.WaitForExit();
var exitCode = process.ExitCode;

process.Close();

var createdFiles =
Expand All @@ -155,8 +222,8 @@ void writeEnvironmentFile(
return (new ProcessOutput
(
ExitCode: exitCode,
StandardError: standardError,
StandardOutput: standardOutput
StandardError: standardErrorBuilder.ToString(),
StandardOutput: standardOutputBuilder.ToString()
), createdFiles);
}

Expand Down

0 comments on commit a70ffd7

Please sign in to comment.