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

Commit c2558f1

Browse files
stephentoubstephentoub
committed
Remove dead code from MemoryMappedFiles implementation
- Resources that are no longer used - Null checks for things that can never be null - Code built into the Windows implementation but only used in the Unix implementation - Branches that can't be hit with a valid implementation
1 parent 42e940f commit c2558f1

11 files changed

+79
-127
lines changed

‎src/System.IO.MemoryMappedFiles/src/Microsoft/Win32/SafeMemoryMappedFileHandle.Windows.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@ internal SafeMemoryMappedFileHandle()
1414
{
1515
}
1616

17-
internal SafeMemoryMappedFileHandle(IntPtr handle, bool ownsHandle)
18-
: base(IntPtr.Zero, ownsHandle)
19-
{
20-
SetHandle(handle);
21-
}
22-
2317
protected override bool ReleaseHandle()
2418
{
2519
return Interop.mincore.CloseHandle(handle);

‎src/System.IO.MemoryMappedFiles/src/Microsoft/Win32/SafeMemoryMappedViewHandle.Unix.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ namespace Microsoft.Win32.SafeHandles
77
{
88
public sealed partial class SafeMemoryMappedViewHandle
99
{
10+
internal SafeMemoryMappedViewHandle(IntPtr handle, bool ownsHandle)
11+
: base(ownsHandle)
12+
{
13+
base.SetHandle(handle);
14+
}
15+
1016
protected override bool ReleaseHandle()
1117
{
1218
IntPtr addr = handle;

‎src/System.IO.MemoryMappedFiles/src/Microsoft/Win32/SafeMemoryMappedViewHandle.Windows.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,9 @@ public sealed partial class SafeMemoryMappedViewHandle
1010
{
1111
protected override bool ReleaseHandle()
1212
{
13-
if (Interop.mincore.UnmapViewOfFile(handle) != 0)
14-
{
15-
handle = IntPtr.Zero;
16-
return true;
17-
}
18-
return false;
13+
IntPtr h = handle;
14+
handle = IntPtr.Zero;
15+
return Interop.mincore.UnmapViewOfFile(h) != 0;
1916
}
2017
}
2118
}

‎src/System.IO.MemoryMappedFiles/src/Microsoft/Win32/SafeMemoryMappedViewHandle.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,5 @@ internal SafeMemoryMappedViewHandle()
1616
: base(true)
1717
{
1818
}
19-
20-
internal SafeMemoryMappedViewHandle(IntPtr handle, bool ownsHandle)
21-
: base(ownsHandle)
22-
{
23-
base.SetHandle(handle);
24-
}
2519
}
2620
}

‎src/System.IO.MemoryMappedFiles/src/Resources/Strings.resx

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -132,18 +132,12 @@
132132
<data name="IO_FileExists_Name" xml:space="preserve">
133133
<value>The file '{0}' already exists.</value>
134134
</data>
135-
<data name="IO_NoPermissionToDirectoryName" xml:space="preserve">
136-
<value>&lt;Path discovery permission to the specified directory was denied.&gt;</value>
137-
</data>
138135
<data name="IO_SharingViolation_File" xml:space="preserve">
139136
<value>The process cannot access the file '{0}' because it is being used by another process.</value>
140137
</data>
141138
<data name="IO_SharingViolation_NoFileName" xml:space="preserve">
142139
<value>The process cannot access the file because it is being used by another process.</value>
143140
</data>
144-
<data name="IO_DriveNotFound_Drive" xml:space="preserve">
145-
<value>Could not find the drive '{0}'. The drive might not be ready or might not be mapped.</value>
146-
</data>
147141
<data name="IO_PathNotFound_Path" xml:space="preserve">
148142
<value>Could not find a part of the path '{0}'.</value>
149143
</data>
@@ -204,9 +198,6 @@
204198
<data name="InvalidOperation_CantCreateFileMapping" xml:space="preserve">
205199
<value>Cannot create file mapping.</value>
206200
</data>
207-
<data name="InvalidOperation_ViewIsNull" xml:space="preserve">
208-
<value>The underlying MemoryMappedView object is null.</value>
209-
</data>
210201
<data name="NotSupported_MMViewStreamsFixedLength" xml:space="preserve">
211202
<value>MemoryMappedViewStreams are fixed length.</value>
212203
</data>
@@ -222,9 +213,6 @@
222213
<data name="ObjectDisposed_StreamIsClosed" xml:space="preserve">
223214
<value>Cannot access a closed Stream.</value>
224215
</data>
225-
<data name="UnknownError_Num" xml:space="preserve">
226-
<value>Unknown error '{0}'.</value>
227-
</data>
228216
<data name="PlatformNotSupported_NamedMaps" xml:space="preserve">
229217
<value>Named maps are not supported.</value>
230218
</data>

‎src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ private static SafeMemoryMappedFileHandle CreateCore(
4343
throw Win32Marshal.GetExceptionForWin32Error(errorCode);
4444
}
4545
}
46-
else if (handle.IsInvalid)
46+
else // handle.IsInvalid
4747
{
4848
throw Win32Marshal.GetExceptionForWin32Error(errorCode);
4949
}
@@ -190,8 +190,9 @@ internal static int GetFileMapAccess(MemoryMappedFileAccess access)
190190
case MemoryMappedFileAccess.ReadWrite: return Interop.mincore.FileMapOptions.FILE_MAP_READ | Interop.mincore.FileMapOptions.FILE_MAP_WRITE;
191191
case MemoryMappedFileAccess.CopyOnWrite: return Interop.mincore.FileMapOptions.FILE_MAP_COPY;
192192
case MemoryMappedFileAccess.ReadExecute: return Interop.mincore.FileMapOptions.FILE_MAP_EXECUTE | Interop.mincore.FileMapOptions.FILE_MAP_READ;
193-
case MemoryMappedFileAccess.ReadWriteExecute: return Interop.mincore.FileMapOptions.FILE_MAP_EXECUTE | Interop.mincore.FileMapOptions.FILE_MAP_READ | Interop.mincore.FileMapOptions.FILE_MAP_WRITE;
194-
default: throw new ArgumentOutOfRangeException("access");
193+
default:
194+
Debug.Assert(access == MemoryMappedFileAccess.ReadWriteExecute);
195+
return Interop.mincore.FileMapOptions.FILE_MAP_EXECUTE | Interop.mincore.FileMapOptions.FILE_MAP_READ | Interop.mincore.FileMapOptions.FILE_MAP_WRITE;
195196
}
196197
}
197198

@@ -208,8 +209,9 @@ internal static int GetPageAccess(MemoryMappedFileAccess access)
208209
case MemoryMappedFileAccess.ReadWrite: return Interop.mincore.PageOptions.PAGE_READWRITE;
209210
case MemoryMappedFileAccess.CopyOnWrite: return Interop.mincore.PageOptions.PAGE_WRITECOPY;
210211
case MemoryMappedFileAccess.ReadExecute: return Interop.mincore.PageOptions.PAGE_EXECUTE_READ;
211-
case MemoryMappedFileAccess.ReadWriteExecute: return Interop.mincore.PageOptions.PAGE_EXECUTE_READWRITE;
212-
default: throw new ArgumentOutOfRangeException("access");
212+
default:
213+
Debug.Assert(access == MemoryMappedFileAccess.ReadWriteExecute);
214+
return Interop.mincore.PageOptions.PAGE_EXECUTE_READWRITE;
213215
}
214216
}
215217

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ public partial class MemoryMappedFile : IDisposable
1818
[SecurityCritical]
1919
private MemoryMappedFile(SafeMemoryMappedFileHandle handle)
2020
{
21-
Debug.Assert(handle != null && !handle.IsClosed && !handle.IsInvalid, "handle is null, closed, or invalid");
21+
Debug.Assert(handle != null);
22+
Debug.Assert(!handle.IsClosed);
23+
Debug.Assert(!handle.IsInvalid);
2224

2325
_handle = handle;
2426
_leaveOpen = true; // No FileStream to dispose of in this case.
@@ -27,8 +29,10 @@ private MemoryMappedFile(SafeMemoryMappedFileHandle handle)
2729
[SecurityCritical]
2830
private MemoryMappedFile(SafeMemoryMappedFileHandle handle, FileStream fileStream, bool leaveOpen)
2931
{
30-
Debug.Assert(handle != null && !handle.IsClosed && !handle.IsInvalid, "handle is null, closed, or invalid");
31-
Debug.Assert(fileStream != null, "fileStream is null");
32+
Debug.Assert(handle != null);
33+
Debug.Assert(!handle.IsClosed);
34+
Debug.Assert(!handle.IsInvalid);
35+
Debug.Assert(fileStream != null);
3236

3337
_handle = handle;
3438
_fileStream = fileStream;
@@ -180,7 +184,8 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string
180184
throw;
181185
}
182186

183-
Debug.Assert(handle != null && !handle.IsInvalid);
187+
Debug.Assert(handle != null);
188+
Debug.Assert(!handle.IsInvalid);
184189
return new MemoryMappedFile(handle, fileStream, false);
185190
}
186191

@@ -466,7 +471,7 @@ protected virtual void Dispose(bool disposing)
466471
{
467472
try
468473
{
469-
if (_handle != null && !_handle.IsClosed)
474+
if (!_handle.IsClosed)
470475
{
471476
_handle.Dispose();
472477
}
@@ -496,17 +501,15 @@ internal static FileAccess GetFileAccess(MemoryMappedFileAccess access)
496501
case MemoryMappedFileAccess.Read:
497502
case MemoryMappedFileAccess.ReadExecute:
498503
return FileAccess.Read;
499-
500-
case MemoryMappedFileAccess.Write:
501-
return FileAccess.Write;
502504

503505
case MemoryMappedFileAccess.ReadWrite:
504506
case MemoryMappedFileAccess.CopyOnWrite:
505507
case MemoryMappedFileAccess.ReadWriteExecute:
506508
return FileAccess.ReadWrite;
507-
509+
508510
default:
509-
throw new ArgumentOutOfRangeException("access");
511+
Debug.Assert(access == MemoryMappedFileAccess.Write);
512+
return FileAccess.Write;
510513
}
511514
}
512515

‎src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedView.Windows.cs

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -107,57 +107,54 @@ public unsafe static MemoryMappedView CreateView(SafeMemoryMappedFileHandle memM
107107
[SecurityCritical]
108108
public void Flush(UIntPtr capacity)
109109
{
110-
if (_viewHandle != null)
110+
unsafe
111111
{
112-
unsafe
112+
byte* firstPagePtr = null;
113+
try
113114
{
114-
byte* firstPagePtr = null;
115-
try
116-
{
117-
_viewHandle.AcquirePointer(ref firstPagePtr);
115+
_viewHandle.AcquirePointer(ref firstPagePtr);
118116

119-
bool success = Interop.mincore.FlushViewOfFile((IntPtr)firstPagePtr, capacity) != 0;
120-
if (success)
121-
return; // This will visit the finally block.
117+
bool success = Interop.mincore.FlushViewOfFile((IntPtr)firstPagePtr, capacity) != 0;
118+
if (success)
119+
return; // This will visit the finally block.
122120

123-
// It is a known issue within the NTFS transaction log system that
124-
// causes FlushViewOfFile to intermittently fail with ERROR_LOCK_VIOLATION
125-
// As a workaround, we catch this particular error and retry the flush operation
126-
// a few milliseconds later. If it does not work, we give it a few more tries with
127-
// increasing intervals. Eventually, however, we need to give up. In ad-hoc tests
128-
// this strategy successfully flushed the view after no more than 3 retries.
121+
// It is a known issue within the NTFS transaction log system that
122+
// causes FlushViewOfFile to intermittently fail with ERROR_LOCK_VIOLATION
123+
// As a workaround, we catch this particular error and retry the flush operation
124+
// a few milliseconds later. If it does not work, we give it a few more tries with
125+
// increasing intervals. Eventually, however, we need to give up. In ad-hoc tests
126+
// this strategy successfully flushed the view after no more than 3 retries.
129127

130-
int error = Marshal.GetLastWin32Error();
131-
bool canRetry = (!success && error == Interop.mincore.Errors.ERROR_LOCK_VIOLATION);
128+
int error = Marshal.GetLastWin32Error();
129+
bool canRetry = (!success && error == Interop.mincore.Errors.ERROR_LOCK_VIOLATION);
132130

133-
SpinWait spinWait = new SpinWait();
134-
for (int w = 0; canRetry && w < MaxFlushWaits; w++)
135-
{
136-
int pause = (1 << w); // MaxFlushRetries should never be over 30
137-
MemoryMappedFile.ThreadSleep(pause);
131+
SpinWait spinWait = new SpinWait();
132+
for (int w = 0; canRetry && w < MaxFlushWaits; w++)
133+
{
134+
int pause = (1 << w); // MaxFlushRetries should never be over 30
135+
MemoryMappedFile.ThreadSleep(pause);
138136

139-
for (int r = 0; canRetry && r < MaxFlushRetriesPerWait; r++)
140-
{
141-
success = Interop.mincore.FlushViewOfFile((IntPtr)firstPagePtr, capacity) != 0;
142-
if (success)
143-
return; // This will visit the finally block.
137+
for (int r = 0; canRetry && r < MaxFlushRetriesPerWait; r++)
138+
{
139+
success = Interop.mincore.FlushViewOfFile((IntPtr)firstPagePtr, capacity) != 0;
140+
if (success)
141+
return; // This will visit the finally block.
144142

145-
spinWait.SpinOnce();
143+
spinWait.SpinOnce();
146144

147-
error = Marshal.GetLastWin32Error();
148-
canRetry = (error == Interop.mincore.Errors.ERROR_LOCK_VIOLATION);
149-
}
145+
error = Marshal.GetLastWin32Error();
146+
canRetry = (error == Interop.mincore.Errors.ERROR_LOCK_VIOLATION);
150147
}
151-
152-
// We got to here, so there was no success:
153-
throw Win32Marshal.GetExceptionForWin32Error(error);
154148
}
155-
finally
149+
150+
// We got to here, so there was no success:
151+
throw Win32Marshal.GetExceptionForWin32Error(error);
152+
}
153+
finally
154+
{
155+
if (firstPagePtr != null)
156156
{
157-
if (firstPagePtr != null)
158-
{
159-
_viewHandle.ReleasePointer();
160-
}
157+
_viewHandle.ReleasePointer();
161158
}
162159
}
163160
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ internal partial class MemoryMappedView : IDisposable
1818
private unsafe MemoryMappedView(SafeMemoryMappedViewHandle viewHandle, long pointerOffset,
1919
long size, MemoryMappedFileAccess access)
2020
{
21+
Debug.Assert(viewHandle != null);
22+
2123
_viewHandle = viewHandle;
2224
_pointerOffset = pointerOffset;
2325
_size = size;
@@ -48,7 +50,7 @@ public MemoryMappedFileAccess Access
4850
[SecurityCritical]
4951
protected virtual void Dispose(bool disposing)
5052
{
51-
if (_viewHandle != null && !_viewHandle.IsClosed)
53+
if (!_viewHandle.IsClosed)
5254
{
5355
_viewHandle.Dispose();
5456
}
@@ -64,7 +66,7 @@ public void Dispose()
6466
public bool IsClosed
6567
{
6668
[SecuritySafeCritical]
67-
get { return (_viewHandle == null || _viewHandle.IsClosed); }
69+
get { return _viewHandle.IsClosed; }
6870
}
6971

7072
/// <summary>

‎src/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedViewAccessor.cs

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,12 @@ internal MemoryMappedViewAccessor(MemoryMappedView view)
2323
public SafeMemoryMappedViewHandle SafeMemoryMappedViewHandle
2424
{
2525
[SecurityCritical]
26-
get { return _view != null ? _view.ViewHandle : null; }
26+
get { return _view.ViewHandle; }
2727
}
2828

2929
public long PointerOffset
3030
{
31-
get
32-
{
33-
if (_view == null)
34-
{
35-
throw new InvalidOperationException(SR.InvalidOperation_ViewIsNull);
36-
}
37-
38-
return _view.PointerOffset;
39-
}
31+
get { return _view.PointerOffset; }
4032
}
4133

4234
[SecuritySafeCritical]
@@ -46,7 +38,7 @@ protected override void Dispose(bool disposing)
4638
{
4739
// Explicitly flush the changes. The OS will do this for us anyway, but not until after the
4840
// MemoryMappedFile object itself is closed.
49-
if (disposing && _view != null && !_view.IsClosed)
41+
if (disposing && !_view.IsClosed)
5042
{
5143
Flush();
5244
}
@@ -55,10 +47,7 @@ protected override void Dispose(bool disposing)
5547
{
5648
try
5749
{
58-
if (_view != null)
59-
{
60-
_view.Dispose();
61-
}
50+
_view.Dispose();
6251
}
6352
finally
6453
{
@@ -82,10 +71,7 @@ public void Flush()
8271

8372
unsafe
8473
{
85-
if (_view != null)
86-
{
87-
_view.Flush((UIntPtr)Capacity);
88-
}
74+
_view.Flush((UIntPtr)Capacity);
8975
}
9076
}
9177
}

0 commit comments

Comments
 (0)