Skip to content

Commit 4a207d1

Browse files
authored
Merge pull request #892 from dotnet/oomFix
MSBuild OOM crash fix
2 parents d146204 + 395dfa7 commit 4a207d1

File tree

1 file changed

+70
-18
lines changed

1 file changed

+70
-18
lines changed

src/NerdBank.GitVersioning/ManagedGit/GitPackIndexMappedReader.cs

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4+
#nullable enable
5+
46
using System.Buffers.Binary;
57
using System.Diagnostics;
68
using System.IO.MemoryMappedFiles;
@@ -14,16 +16,19 @@ namespace Nerdbank.GitVersioning.ManagedGit;
1416
public unsafe class GitPackIndexMappedReader : GitPackIndexReader
1517
{
1618
private readonly MemoryMappedFile file;
17-
private readonly MemoryMappedViewAccessor accessor;
1819

1920
// The fanout table consists of
2021
// 256 4-byte network byte order integers.
2122
// The N-th entry of this table records the number of objects in the corresponding pack,
2223
// the first byte of whose object name is less than or equal to N.
2324
private readonly int[] fanoutTable = new int[257];
25+
private readonly ulong fileLength;
2426

25-
private readonly byte* ptr;
2627
private bool initialized;
28+
private MemoryMappedViewAccessor? accessor;
29+
private ulong accessorOffset;
30+
private ulong accessorSize;
31+
private byte* accessorPtr;
2732

2833
/// <summary>
2934
/// Initializes a new instance of the <see cref="GitPackIndexMappedReader"/> class.
@@ -38,17 +43,8 @@ public GitPackIndexMappedReader(FileStream stream)
3843
throw new ArgumentNullException(nameof(stream));
3944
}
4045

46+
this.fileLength = (ulong)stream.Length;
4147
this.file = MemoryMappedFile.CreateFromFile(stream, mapName: null, capacity: 0, MemoryMappedFileAccess.Read, HandleInheritability.None, leaveOpen: false);
42-
this.accessor = this.file.CreateViewAccessor(0, 0, MemoryMappedFileAccess.Read);
43-
this.accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref this.ptr);
44-
}
45-
46-
private ReadOnlySpan<byte> Value
47-
{
48-
get
49-
{
50-
return new ReadOnlySpan<byte>(this.ptr, (int)this.accessor.Capacity);
51-
}
5248
}
5349

5450
/// <inheritdoc/>
@@ -69,7 +65,7 @@ public override (long? Offset, GitObjectId? ObjectId) GetOffset(Span<byte> objec
6965
int order = 0;
7066

7167
int tableSize = 20 * (packEnd - packStart + 1);
72-
ReadOnlySpan<byte> table = this.Value.Slice(4 + 4 + (256 * 4) + (20 * packStart), tableSize);
68+
ReadOnlySpan<byte> table = this.GetSpan((ulong)(4 + 4 + (256 * 4) + (20 * packStart)), tableSize);
7369

7470
int originalPackStart = packStart;
7571

@@ -117,7 +113,7 @@ public override (long? Offset, GitObjectId? ObjectId) GetOffset(Span<byte> objec
117113
// Get the offset value. It's located at:
118114
// 4 (header) + 4 (version) + 256 * 4 (fanout table) + 20 * objectCount (SHA1 object name table) + 4 * objectCount (CRC32) + 4 * i (offset values)
119115
int offsetTableStart = 4 + 4 + (256 * 4) + (20 * objectCount) + (4 * objectCount);
120-
ReadOnlySpan<byte> offsetBuffer = this.Value.Slice(offsetTableStart + (4 * (i + originalPackStart)), 4);
116+
ReadOnlySpan<byte> offsetBuffer = this.GetSpan((ulong)(offsetTableStart + (4 * (i + originalPackStart))), 4);
121117
uint offset = BinaryPrimitives.ReadUInt32BigEndian(offsetBuffer);
122118

123119
if (offsetBuffer[0] < 128)
@@ -130,7 +126,7 @@ public override (long? Offset, GitObjectId? ObjectId) GetOffset(Span<byte> objec
130126
// which follows the table of 4-byte offset entries: "large offsets are encoded as an index into the next table with the msbit set."
131127
offset = offset & 0x7FFFFFFF;
132128

133-
offsetBuffer = this.Value.Slice(offsetTableStart + (4 * objectCount) + (8 * (int)offset), 8);
129+
offsetBuffer = this.GetSpan((ulong)(offsetTableStart + (4 * objectCount) + (8 * (int)offset)), 8);
134130
long offset64 = BinaryPrimitives.ReadInt64BigEndian(offsetBuffer);
135131
return (offset64, GitObjectId.Parse(table.Slice(20 * i, 20)));
136132
}
@@ -139,22 +135,78 @@ public override (long? Offset, GitObjectId? ObjectId) GetOffset(Span<byte> objec
139135
/// <inheritdoc/>
140136
public override void Dispose()
141137
{
142-
this.accessor.Dispose();
138+
if (this.accessorPtr is not null && this.accessor is not null)
139+
{
140+
this.accessor.SafeMemoryMappedViewHandle.ReleasePointer();
141+
this.accessorPtr = null;
142+
}
143+
144+
this.accessor?.Dispose();
145+
this.accessor = null;
143146
this.file.Dispose();
144147
}
145148

149+
private unsafe ReadOnlySpan<byte> GetSpan(ulong offset, int length)
150+
{
151+
checked
152+
{
153+
// If the request is for a window that we have not currently mapped, throw away what we have.
154+
if (this.accessor is not null && (this.accessorOffset > offset || this.accessorOffset + this.accessorSize < offset + (ulong)length))
155+
{
156+
if (this.accessorPtr is not null)
157+
{
158+
this.accessor.SafeMemoryMappedViewHandle.ReleasePointer();
159+
this.accessorPtr = null;
160+
}
161+
162+
this.accessor.Dispose();
163+
this.accessor = null;
164+
}
165+
166+
if (this.accessor is null)
167+
{
168+
const int minimumLength = 10 * 1024 * 1024;
169+
uint windowSize = (uint)Math.Min((ulong)Math.Max(minimumLength, length), this.fileLength);
170+
171+
// Push window 'to the left' if our preferred minimum size doesn't fit when we start at the offset requested.
172+
ulong actualOffset = offset + windowSize > this.fileLength ? this.fileLength - windowSize : offset;
173+
174+
this.accessor = this.file.CreateViewAccessor((long)actualOffset, windowSize, MemoryMappedFileAccess.Read);
175+
176+
// Record the *actual* offset into the file that the pointer to native memory points at.
177+
// This may be earlier in the file than we requested, and if so, go ahead and take advantage of that.
178+
this.accessorOffset = actualOffset - (ulong)this.accessor.PointerOffset;
179+
180+
// Also record the *actual* length of the mapped memory, again so we can take full advantage before reallocating the view.
181+
this.accessorSize = this.accessor.SafeMemoryMappedViewHandle.ByteLength;
182+
}
183+
184+
Debug.Assert(offset >= (ulong)this.accessor.PointerOffset);
185+
byte* ptr = this.accessorPtr;
186+
if (ptr is null)
187+
{
188+
this.accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref this.accessorPtr);
189+
ptr = this.accessorPtr;
190+
}
191+
192+
ptr += offset - this.accessorOffset;
193+
return new ReadOnlySpan<byte>(ptr, length);
194+
}
195+
}
196+
146197
private void Initialize()
147198
{
148199
if (!this.initialized)
149200
{
150-
ReadOnlySpan<byte> value = this.Value;
201+
const int fanoutTableLength = 256;
202+
ReadOnlySpan<byte> value = this.GetSpan(0, 4 + (4 * fanoutTableLength) + 4);
151203

152204
ReadOnlySpan<byte> header = value.Slice(0, 4);
153205
int version = BinaryPrimitives.ReadInt32BigEndian(value.Slice(4, 4));
154206
Debug.Assert(header.SequenceEqual(Header));
155207
Debug.Assert(version == 2);
156208

157-
for (int i = 1; i <= 256; i++)
209+
for (int i = 1; i <= fanoutTableLength; i++)
158210
{
159211
this.fanoutTable[i] = BinaryPrimitives.ReadInt32BigEndian(value.Slice(4 + (4 * i), 4));
160212
}

0 commit comments

Comments
 (0)