From 872d9e47d1c3fe7e4ad262340a548977c0d46ce1 Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Wed, 3 Mar 2021 07:06:19 -0800 Subject: [PATCH] HostWriter Performance Improvements (#48774) --- .../AppHost/BinaryUtils.cs | 24 +++ .../AppHost/HostWriter.cs | 79 ++++---- .../AppHost/MachOUtils.cs | 177 +++++++++--------- .../AppHostUpdateTests.cs | 4 +- 4 files changed, 156 insertions(+), 128 deletions(-) diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/BinaryUtils.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/BinaryUtils.cs index d275278955cdc7..0d733815e625b8 100644 --- a/src/installer/managed/Microsoft.NET.HostModel/AppHost/BinaryUtils.cs +++ b/src/installer/managed/Microsoft.NET.HostModel/AppHost/BinaryUtils.cs @@ -173,5 +173,29 @@ public static void CopyFile(string sourcePath, string destinationPath) // Copy file to destination path so it inherits the same attributes/permissions. File.Copy(sourcePath, destinationPath, overwrite: true); } + + internal static void WriteToStream(MemoryMappedViewAccessor sourceViewAccessor, FileStream fileStream) + { + int pos = 0; + int bufSize = 16384; //16K + + byte[] buf = new byte[bufSize]; + do + { + int bytesRequested = Math.Min((int)sourceViewAccessor.Capacity - pos, bufSize); + if (bytesRequested <= 0) + { + break; + } + + int bytesRead = sourceViewAccessor.ReadArray(pos, buf, 0, bytesRequested); + if (bytesRead > 0) + { + fileStream.Write(buf, 0, bytesRead); + pos += bytesRead; + } + } + while (true); + } } } diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs index 6c20515db7240c..6ff8ec44e27f36 100644 --- a/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs +++ b/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs @@ -44,31 +44,23 @@ public static void CreateAppHost( throw new AppNameTooLongException(appBinaryFilePath); } - BinaryUtils.CopyFile(appHostSourceFilePath, appHostDestinationFilePath); - bool appHostIsPEImage = false; - void RewriteAppHost() + void RewriteAppHost(MemoryMappedViewAccessor accessor) { // Re-write the destination apphost with the proper contents. - using (var memoryMappedFile = MemoryMappedFile.CreateFromFile(appHostDestinationFilePath)) - { - using (MemoryMappedViewAccessor accessor = memoryMappedFile.CreateViewAccessor()) - { - BinaryUtils.SearchAndReplace(accessor, AppBinaryPathPlaceholderSearchValue, bytesToWrite); - - appHostIsPEImage = PEUtils.IsPEImage(accessor); + BinaryUtils.SearchAndReplace(accessor, AppBinaryPathPlaceholderSearchValue, bytesToWrite); - if (windowsGraphicalUserInterface) - { - if (!appHostIsPEImage) - { - throw new AppHostNotPEFileException(); - } + appHostIsPEImage = PEUtils.IsPEImage(accessor); - PEUtils.SetWindowsGraphicalUserInterfaceBit(accessor); - } + if (windowsGraphicalUserInterface) + { + if (!appHostIsPEImage) + { + throw new AppHostNotPEFileException(); } + + PEUtils.SetWindowsGraphicalUserInterfaceBit(accessor); } } @@ -90,19 +82,42 @@ void UpdateResources() } } - void RemoveSignatureIfMachO() + try { - MachOUtils.RemoveSignature(appHostDestinationFilePath); - } + RetryUtil.RetryOnIOError(() => + { + MemoryMappedFile memoryMappedFile = null; + MemoryMappedViewAccessor memoryMappedViewAccessor = null; + try + { + // Open the source host file. + memoryMappedFile = MemoryMappedFile.CreateFromFile(appHostSourceFilePath, FileMode.Open, null, 0, MemoryMappedFileAccess.CopyOnWrite); + memoryMappedViewAccessor = memoryMappedFile.CreateViewAccessor(0, 0, MemoryMappedFileAccess.CopyOnWrite); - void SetLastWriteTime() - { - // Memory-mapped write does not updating last write time - File.SetLastWriteTimeUtc(appHostDestinationFilePath, DateTime.UtcNow); - } + // Transform the host file in-memory. + RewriteAppHost(memoryMappedViewAccessor); + + // Save the transformed host. + using (FileStream fileStream = new FileStream(appHostDestinationFilePath, FileMode.Create)) + { + BinaryUtils.WriteToStream(memoryMappedViewAccessor, fileStream); + + // Remove the signature from MachO hosts. + if (!appHostIsPEImage) + { + MachOUtils.RemoveSignature(fileStream); + } + } + } + finally + { + memoryMappedViewAccessor?.Dispose(); + memoryMappedFile?.Dispose(); + } + }); + + RetryUtil.RetryOnWin32Error(UpdateResources); - try - { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { var filePermissionOctal = Convert.ToInt32("755", 8); // -rwxr-xr-x @@ -120,14 +135,6 @@ void SetLastWriteTime() throw new Win32Exception(Marshal.GetLastWin32Error(), $"Could not set file permission {filePermissionOctal} for {appHostDestinationFilePath}."); } } - - RetryUtil.RetryOnIOError(RewriteAppHost); - - RetryUtil.RetryOnWin32Error(UpdateResources); - - RetryUtil.RetryOnIOError(RemoveSignatureIfMachO); - - RetryUtil.RetryOnIOError(SetLastWriteTime); } catch (Exception ex) { diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/MachOUtils.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/MachOUtils.cs index cf2d88bc867204..c717ef75659f9c 100644 --- a/src/installer/managed/Microsoft.NET.HostModel/AppHost/MachOUtils.cs +++ b/src/installer/managed/Microsoft.NET.HostModel/AppHost/MachOUtils.cs @@ -188,7 +188,7 @@ public static bool IsMachOImage(string filePath) /// - Truncates the apphost file to the end of the __LINKEDIT segment /// /// - /// Path to the AppHost + /// Stream containing the AppHost /// /// True if /// - The input is a MachO binary, and @@ -199,124 +199,121 @@ public static bool IsMachOImage(string filePath) /// /// The input is a MachO file, but doesn't match the expect format of the AppHost. /// - public static unsafe bool RemoveSignature(string filePath) + public static unsafe bool RemoveSignature(FileStream stream) { - using (var stream = new FileStream(filePath, FileMode.Open, FileAccess.ReadWrite)) + uint signatureSize = 0; + using (var mappedFile = MemoryMappedFile.CreateFromFile(stream, + mapName: null, + capacity: 0, + MemoryMappedFileAccess.ReadWrite, + HandleInheritability.None, + leaveOpen: true)) { - uint signatureSize = 0; - using (var mappedFile = MemoryMappedFile.CreateFromFile(stream, - mapName: null, - capacity: 0, - MemoryMappedFileAccess.ReadWrite, - HandleInheritability.None, - leaveOpen: true)) + using (var accessor = mappedFile.CreateViewAccessor()) { - using (var accessor = mappedFile.CreateViewAccessor()) + byte* file = null; + try { - byte* file = null; - try - { - accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref file); - Verify(file != null, MachOFormatError.MemoryMapAccessFault); + accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref file); + Verify(file != null, MachOFormatError.MemoryMapAccessFault); - MachHeader* header = (MachHeader*)file; + MachHeader* header = (MachHeader*)file; - if (!header->IsValid()) - { - // Not a MachO file. - return false; - } + if (!header->IsValid()) + { + // Not a MachO file. + return false; + } - Verify(header->Is64BitExecutable(), MachOFormatError.Not64BitExe); + Verify(header->Is64BitExecutable(), MachOFormatError.Not64BitExe); - file += sizeof(MachHeader); - SegmentCommand64* linkEdit = null; - SymtabCommand* symtab = null; - LinkEditDataCommand* signature = null; + file += sizeof(MachHeader); + SegmentCommand64* linkEdit = null; + SymtabCommand* symtab = null; + LinkEditDataCommand* signature = null; - for (uint i = 0; i < header->ncmds; i++) + for (uint i = 0; i < header->ncmds; i++) + { + LoadCommand* command = (LoadCommand*)file; + if (command->cmd == Command.LC_SEGMENT_64) { - LoadCommand* command = (LoadCommand*)file; - if (command->cmd == Command.LC_SEGMENT_64) - { - SegmentCommand64* segment = (SegmentCommand64*)file; - if (segment->SegName.Equals("__LINKEDIT")) - { - Verify(linkEdit == null, MachOFormatError.DuplicateLinkEdit); - linkEdit = segment; - } - } - else if (command->cmd == Command.LC_SYMTAB) - { - Verify(symtab == null, MachOFormatError.DuplicateSymtab); - symtab = (SymtabCommand*)command; - } - else if (command->cmd == Command.LC_CODE_SIGNATURE) + SegmentCommand64* segment = (SegmentCommand64*)file; + if (segment->SegName.Equals("__LINKEDIT")) { - Verify(i == header->ncmds - 1, MachOFormatError.SignCommandNotLast); - signature = (LinkEditDataCommand*)command; - break; + Verify(linkEdit == null, MachOFormatError.DuplicateLinkEdit); + linkEdit = segment; } - - file += command->cmdsize; } - - if (signature != null) + else if (command->cmd == Command.LC_SYMTAB) { - Verify(linkEdit != null, MachOFormatError.MissingLinkEdit); - Verify(symtab != null, MachOFormatError.MissingSymtab); + Verify(symtab == null, MachOFormatError.DuplicateSymtab); + symtab = (SymtabCommand*)command; + } + else if (command->cmd == Command.LC_CODE_SIGNATURE) + { + Verify(i == header->ncmds - 1, MachOFormatError.SignCommandNotLast); + signature = (LinkEditDataCommand*)command; + break; + } - var symtabEnd = symtab->stroff + symtab->strsize; - var linkEditEnd = linkEdit->fileoff + linkEdit->filesize; - var signatureEnd = signature->dataoff + signature->datasize; - var fileEnd = (ulong)stream.Length; + file += command->cmdsize; + } - Verify(linkEditEnd == fileEnd, MachOFormatError.LinkEditNotLast); - Verify(signatureEnd == fileEnd, MachOFormatError.SignBlobNotLast); + if (signature != null) + { + Verify(linkEdit != null, MachOFormatError.MissingLinkEdit); + Verify(symtab != null, MachOFormatError.MissingSymtab); - Verify(symtab->symoff > linkEdit->fileoff, MachOFormatError.SymtabNotInLinkEdit); - Verify(signature->dataoff > linkEdit->fileoff, MachOFormatError.SignNotInLinkEdit); + var symtabEnd = symtab->stroff + symtab->strsize; + var linkEditEnd = linkEdit->fileoff + linkEdit->filesize; + var signatureEnd = signature->dataoff + signature->datasize; + var fileEnd = (ulong)stream.Length; - // The signature blob immediately follows the symtab blob, - // except for a few bytes of padding. - Verify(signature->dataoff >= symtabEnd && signature->dataoff - symtabEnd < 32, MachOFormatError.SignBlobNotLast); + Verify(linkEditEnd == fileEnd, MachOFormatError.LinkEditNotLast); + Verify(signatureEnd == fileEnd, MachOFormatError.SignBlobNotLast); - // Remove the signature command - header->ncmds--; - header->sizeofcmds -= signature->cmdsize; - Unsafe.InitBlock(signature, 0, signature->cmdsize); + Verify(symtab->symoff > linkEdit->fileoff, MachOFormatError.SymtabNotInLinkEdit); + Verify(signature->dataoff > linkEdit->fileoff, MachOFormatError.SignNotInLinkEdit); - // Remove the signature blob (note for truncation) - signatureSize = (uint)(fileEnd - symtabEnd); + // The signature blob immediately follows the symtab blob, + // except for a few bytes of padding. + Verify(signature->dataoff >= symtabEnd && signature->dataoff - symtabEnd < 32, MachOFormatError.SignBlobNotLast); - // Adjust the __LINKEDIT segment load command - linkEdit->filesize -= signatureSize; + // Remove the signature command + header->ncmds--; + header->sizeofcmds -= signature->cmdsize; + Unsafe.InitBlock(signature, 0, signature->cmdsize); - // codesign --remove-signature doesn't reset the vmsize. - // Setting the vmsize here makes the output bin-equal with the original - // unsigned apphost (and not bin-equal with a signed-unsigned-apphost). - linkEdit->vmsize = linkEdit->filesize; - } + // Remove the signature blob (note for truncation) + signatureSize = (uint)(fileEnd - symtabEnd); + + // Adjust the __LINKEDIT segment load command + linkEdit->filesize -= signatureSize; + + // codesign --remove-signature doesn't reset the vmsize. + // Setting the vmsize here makes the output bin-equal with the original + // unsigned apphost (and not bin-equal with a signed-unsigned-apphost). + linkEdit->vmsize = linkEdit->filesize; } - finally + } + finally + { + if (file != null) { - if (file != null) - { - accessor.SafeMemoryMappedViewHandle.ReleasePointer(); - } + accessor.SafeMemoryMappedViewHandle.ReleasePointer(); } } } + } - if (signatureSize != 0) - { - // The signature was removed, update the file length - stream.SetLength(stream.Length - signatureSize); - return true; - } - - return false; + if (signatureSize != 0) + { + // The signature was removed, update the file length + stream.SetLength(stream.Length - signatureSize); + return true; } + + return false; } /// diff --git a/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs b/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs index 5d4466de963dfb..992506c37293bc 100644 --- a/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs +++ b/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs @@ -175,7 +175,7 @@ public void ItGeneratesExecutableImage() string destinationFilePath = Path.Combine(testDirectory.Path, "DestinationAppHost.exe.mock"); string appBinaryFilePath = "Test/App/Binary/Path.dll"; - chmod(sourceAppHostMock, Convert.ToInt32("444", 8)) // make it readonly: -r--r--r-- + chmod(sourceAppHostMock, Convert.ToInt32("755", 8)) // match installed permissions: -rwxr-xr-x .Should() .NotBe(-1); @@ -185,7 +185,7 @@ public void ItGeneratesExecutableImage() GetFilePermissionValue(sourceAppHostMock) .Should() - .Be(Convert.ToInt32("444", 8)); + .Be(Convert.ToInt32("755", 8)); HostWriter.CreateAppHost( sourceAppHostMock,