Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 42e940f

Browse files
committed
Fix three bugs in Unix MemoryMappedFiles implementation
- When a map is created with CreateFromFile and a capacity is specified to CreateFromFile that's larger than the file using to back the map, the file's length is increased to match. If the capacity is too large to successfully increase the length of the file, on Windows an IOException results, but on Unix we were throwing an ArgumentException. The fix just catches the ArgumentException and translates it to an IOException. This should be a very rare occurence, even for an exception case, and the additional overheads involved in the catch and throw-new should be immaterial. - It's possible to create a map view with a size of zero (over a map with a non-zero size). In some of these cases, we were passing invalid data to the underlying SafeBuffer that gets created, resulting in exceptions for what should be valid cases. The fix is to reset some values appropriately to reflect the 0-size special case (there's already a special case for this in the code, we just weren't configuring everything that needed to be configured). - When we create the backing stores for anonymous maps, we first create the object (memory or file) and then attempt to resize it. In the case of an extremely large capacity, it's possible the resize could fail. We weren't deterministically then cleaning up the handle that had just been recreated, instead waiting for finalization to do so. This commit fixes that.
1 parent 6f1a32c commit 42e940f

File tree

4 files changed

+44
-17
lines changed

4 files changed

+44
-17
lines changed

src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.BackingObject_File.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,16 @@ private static FileStream CreateSharedBackingObject(Interop.libc.MemoryMappedPro
2020
// Then enlarge it to the requested capacity.
2121
const int DefaultBufferSize = 0x1000;
2222
var fs = new FileStream(path, FileMode.CreateNew, TranslateProtectionsToFileAccess(protections), FileShare.ReadWrite, DefaultBufferSize);
23-
Interop.CheckIo(Interop.libc.unlink(path));
24-
fs.SetLength(capacity);
23+
try
24+
{
25+
Interop.CheckIo(Interop.libc.unlink(path));
26+
fs.SetLength(capacity);
27+
}
28+
catch
29+
{
30+
fs.Dispose();
31+
throw;
32+
}
2533
return fs;
2634
}
2735

src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.BackingObject_Memory.cs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,27 @@ private static FileStream CreateSharedBackingObject(
3232
int fd;
3333
Interop.CheckIo(fd = Interop.libc.shm_open(mapName, flags, (int)perms), mapName);
3434
SafeFileHandle fileHandle = new SafeFileHandle((IntPtr)fd, ownsHandle: true);
35-
36-
// Unlink the shared memory object immediatley so that it'll go away once all handles
37-
// to it are closed (as with opened then unlinked files, it'll remain usable via
38-
// the open handles even though it's unlinked and can't be opened anew via its name).
39-
Interop.CheckIo(Interop.libc.shm_unlink(mapName));
40-
41-
// Give it the right capacity. We do this directly with ftruncate rather
42-
// than via FileStream.SetLength after the FileStream is created because, on some systems,
43-
// lseek fails on shared memory objects, causing the FileStream to think it's unseekable,
44-
// causing it to preemptively throw from SetLength.
45-
Interop.CheckIo(Interop.libc.ftruncate(fd, capacity));
46-
47-
// Wrap the file descriptor in a stream and return it.
48-
return new FileStream(fileHandle, TranslateProtectionsToFileAccess(protections));
35+
try
36+
{
37+
// Unlink the shared memory object immediatley so that it'll go away once all handles
38+
// to it are closed (as with opened then unlinked files, it'll remain usable via
39+
// the open handles even though it's unlinked and can't be opened anew via its name).
40+
Interop.CheckIo(Interop.libc.shm_unlink(mapName));
41+
42+
// Give it the right capacity. We do this directly with ftruncate rather
43+
// than via FileStream.SetLength after the FileStream is created because, on some systems,
44+
// lseek fails on shared memory objects, causing the FileStream to think it's unseekable,
45+
// causing it to preemptively throw from SetLength.
46+
Interop.CheckIo(Interop.libc.ftruncate(fd, capacity));
47+
48+
// Wrap the file descriptor in a stream and return it.
49+
return new FileStream(fileHandle, TranslateProtectionsToFileAccess(protections));
50+
}
51+
catch
52+
{
53+
fileHandle.Dispose();
54+
throw;
55+
}
4956
}
5057
}
5158
}

src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,16 @@ private static unsafe SafeMemoryMappedFileHandle CreateCore(
4040
// at least as big as the requested capacity of the map.
4141
if (fileStream.Length < capacity)
4242
{
43-
fileStream.SetLength(capacity);
43+
try
44+
{
45+
fileStream.SetLength(capacity);
46+
}
47+
catch (ArgumentException exc)
48+
{
49+
// If the capacity is too large, we'll get an ArgumentException from SetLength,
50+
// but on Windows this same condition is represented by an IOException.
51+
throw new IOException(exc.Message, exc);
52+
}
4453
}
4554
}
4655
else

src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedView.Unix.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public unsafe static MemoryMappedView CreateView(
4444
// the right size and not extending the size to be page-aligned.)
4545
ulong nativeSize, extraMemNeeded, nativeOffset;
4646
int pageSize = Interop.libc.sysconf(Interop.libc.SysConfNames._SC_PAGESIZE);
47+
Debug.Assert(pageSize > 0);
4748
ValidateSizeAndOffset(
4849
requestedSize, requestedOffset, pageSize,
4950
out nativeSize, out extraMemNeeded, out nativeOffset);
@@ -115,6 +116,8 @@ public unsafe static MemoryMappedView CreateView(
115116
flags | Interop.libc.MemoryMappedFlags.MAP_ANONYMOUS,
116117
-1, // ignore the actual fd even if there was one
117118
0);
119+
requestedSize = 0;
120+
extraMemNeeded = 0;
118121
}
119122
if ((long)addr < 0)
120123
{

0 commit comments

Comments
 (0)