Skip to content

Commit

Permalink
Fix binlog OOM embedding files
Browse files Browse the repository at this point in the history
Fixes #8595 by storing the embedded-file zip in a temporary directory
(instead of memory or binlog target directory) to avoid problems with
file watchers.
  • Loading branch information
rainersigwald committed Jul 12, 2023
1 parent 5785ed5 commit 4cf11b4
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 43 deletions.
18 changes: 16 additions & 2 deletions src/Build/Logging/BinaryLogger/BinaryLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.IO.Compression;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using Microsoft.Build.Shared.FileSystem;

#nullable disable

Expand Down Expand Up @@ -226,12 +227,25 @@ public void Shutdown()

if (projectImportsCollector != null)
{
projectImportsCollector.Close();

if (CollectProjectImports == ProjectImportsCollectionMode.Embed)
{
eventArgsWriter.WriteBlob(BinaryLogRecordKind.ProjectImportArchive, projectImportsCollector.GetAllBytes());
var archiveFilePath = projectImportsCollector.ArchiveFilePath;

// It is possible that the archive couldn't be created for some reason.
// Only embed it if it actually exists.
if (FileSystems.Default.FileExists(archiveFilePath))
{
using (FileStream fileStream = File.OpenRead(archiveFilePath))
{
eventArgsWriter.WriteBlob(BinaryLogRecordKind.ProjectImportArchive, fileStream);
}

File.Delete(archiveFilePath);
}
}

projectImportsCollector.Close();
projectImportsCollector = null;
}

Expand Down
16 changes: 16 additions & 0 deletions src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,17 @@ public void WriteBlob(BinaryLogRecordKind kind, byte[] bytes)
Write(bytes);
}

public void WriteBlob(BinaryLogRecordKind kind, Stream stream)
{
// write the blob directly to the underlying writer,
// bypassing the memory stream
using var redirection = RedirectWritesToOriginalWriter();

Write(kind);
Write(stream.Length);
Write(stream);
}

/// <summary>
/// Switches the binaryWriter used by the Write* methods to the direct underlying stream writer
/// until the disposable is disposed. Useful to bypass the currentRecordWriter to write a string,
Expand Down Expand Up @@ -1091,6 +1102,11 @@ private void Write(byte[] bytes)
binaryWriter.Write(bytes);
}

private void Write(Stream stream)
{
stream.CopyTo(binaryWriter.BaseStream);
}

private void Write(byte b)
{
binaryWriter.Write(b);
Expand Down
74 changes: 34 additions & 40 deletions src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.IO.Compression;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Build.Shared;

#nullable disable

Expand All @@ -20,30 +21,10 @@ namespace Microsoft.Build.Logging
/// </summary>
internal class ProjectImportsCollector
{
private Stream _stream;
public byte[] GetAllBytes()
{
if (_stream == null)
{
return Array.Empty<byte>();
}
else if (ArchiveFilePath == null)
{
var stream = _stream as MemoryStream;
// Before we can use the zip archive, it must be closed.
Close(false);
return stream.ToArray();
}
else
{
Close();
return File.ReadAllBytes(ArchiveFilePath);
}
}

private Stream _fileStream;
private ZipArchive _zipArchive;

private string ArchiveFilePath { get; set; }
public string ArchiveFilePath { get; }

/// <summary>
/// Avoid visiting each file more than once.
Expand All @@ -55,33 +36,46 @@ public byte[] GetAllBytes()

public ProjectImportsCollector(string logFilePath, bool createFile, string sourcesArchiveExtension = ".ProjectImports.zip")
{
try
if (createFile)
{
if (createFile)
{
ArchiveFilePath = Path.ChangeExtension(logFilePath, sourcesArchiveExtension);
_stream = new FileStream(ArchiveFilePath, FileMode.Create, FileAccess.ReadWrite, FileShare.Delete);
}
else
// Archive file will be stored alongside the binlog
ArchiveFilePath = Path.ChangeExtension(logFilePath, sourcesArchiveExtension);
}
else
{
string cacheDirectory = FileUtilities.GetCacheDirectory();
if (!Directory.Exists(cacheDirectory))
{
_stream = new MemoryStream();
Directory.CreateDirectory(cacheDirectory);
}
_zipArchive = new ZipArchive(_stream, ZipArchiveMode.Create, true);

// Archive file will be temporarily stored in MSBuild cache folder and deleted when no longer needed
ArchiveFilePath = Path.Combine(
cacheDirectory,
Path.ChangeExtension(
Path.GetFileName(logFilePath),
sourcesArchiveExtension));
}

try
{
_fileStream = new FileStream(ArchiveFilePath, FileMode.Create, FileAccess.ReadWrite, FileShare.Delete);
_zipArchive = new ZipArchive(_fileStream, ZipArchiveMode.Create);
}
catch
{
// For some reason we weren't able to create a file for the archive.
// Disable the file collector.
_stream = null;
_fileStream = null;
_zipArchive = null;
}
}

public void AddFile(string filePath)
{
if (filePath != null && _stream != null)
if (filePath != null && _fileStream != null)
{
lock (_stream)
lock (_fileStream)
{
// enqueue the task to add a file and return quickly
// to avoid holding up the current thread
Expand All @@ -101,9 +95,9 @@ public void AddFile(string filePath)

public void AddFileFromMemory(string filePath, string data)
{
if (filePath != null && data != null && _stream != null)
if (filePath != null && data != null && _fileStream != null)
{
lock (_stream)
lock (_fileStream)
{
// enqueue the task to add a file and return quickly
// to avoid holding up the current thread
Expand Down Expand Up @@ -197,7 +191,7 @@ private static string CalculateArchivePath(string filePath)
return archivePath;
}

public void Close(bool closeStream = true)
public void Close()
{
// wait for all pending file writes to complete
_currentTask.Wait();
Expand All @@ -208,10 +202,10 @@ public void Close(bool closeStream = true)
_zipArchive = null;
}

if (closeStream && (_stream != null))
if (_fileStream != null)
{
_stream.Dispose();
_stream = null;
_fileStream.Dispose();
_fileStream = null;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/MSBuild/XMake.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1498,8 +1498,8 @@ internal static bool BuildProject(
}
finally
{
FileUtilities.ClearCacheDirectory();
projectCollection?.Dispose();
FileUtilities.ClearCacheDirectory();

// Build manager shall be reused for all build sessions.
// If, for one reason or another, this behavior needs to change in future
Expand Down

0 comments on commit 4cf11b4

Please sign in to comment.