From 4f18b045f82d87eaca4ae2df146842df7f309361 Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Wed, 22 Nov 2017 09:29:44 -0800 Subject: [PATCH 1/7] Initial fix implementation. Almost certainly has some issues. --- .../Windows/HttpApi/Interop.HttpApi.cs | 27 +++++++--------- .../System/Net/Windows/AsyncRequestContext.cs | 4 +-- .../Windows/HttpListenerRequest.Windows.cs | 2 +- .../System/Net/Windows/RequestContextBase.cs | 32 +++++++++++++------ .../System/Net/Windows/SyncRequestContext.cs | 6 ++-- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs index a4654b51ab44..c0f056c18f0c 100644 --- a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs +++ b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs @@ -786,23 +786,20 @@ internal static unsafe string GetVerb(HTTP_REQUEST* request) return GetVerb(request, 0); } - internal static unsafe string GetVerb(byte[] memoryBlob, IntPtr originalAddress) + internal static unsafe string GetVerb(IntPtr memoryBlob, IntPtr originalAddress) { - fixed (byte* pMemoryBlob = memoryBlob) - { - return GetVerb((HTTP_REQUEST*)pMemoryBlob, pMemoryBlob - (byte*)originalAddress); - } + return GetVerb((HTTP_REQUEST*)memoryBlob.ToPointer(), (byte*)memoryBlob - (byte*)originalAddress); } // Server API - internal static unsafe WebHeaderCollection GetHeaders(byte[] memoryBlob, IntPtr originalAddress) + internal static unsafe WebHeaderCollection GetHeaders(IntPtr memoryBlob, IntPtr originalAddress) { NetEventSource.Enter(null); // Return value. WebHeaderCollection headerCollection = new WebHeaderCollection(); - fixed (byte* pMemoryBlob = memoryBlob) + byte* pMemoryBlob = (byte*)memoryBlob; { HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; long fixup = pMemoryBlob - (byte*)originalAddress; @@ -854,7 +851,7 @@ internal static unsafe WebHeaderCollection GetHeaders(byte[] memoryBlob, IntPtr } - internal static unsafe uint GetChunks(byte[] memoryBlob, IntPtr originalAddress, ref int dataChunkIndex, ref uint dataChunkOffset, byte[] buffer, int offset, int size) + internal static unsafe uint GetChunks(IntPtr memoryBlob, IntPtr originalAddress, ref int dataChunkIndex, ref uint dataChunkOffset, byte[] buffer, int offset, int size) { if (NetEventSource.IsEnabled) { @@ -863,7 +860,7 @@ internal static unsafe uint GetChunks(byte[] memoryBlob, IntPtr originalAddress, // Return value. uint dataRead = 0; - fixed (byte* pMemoryBlob = memoryBlob) + byte* pMemoryBlob = (byte*)memoryBlob; { HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; long fixup = pMemoryBlob - (byte*)originalAddress; @@ -917,13 +914,13 @@ internal static unsafe uint GetChunks(byte[] memoryBlob, IntPtr originalAddress, return dataRead; } - internal static unsafe HTTP_VERB GetKnownVerb(byte[] memoryBlob, IntPtr originalAddress) + internal static unsafe HTTP_VERB GetKnownVerb(IntPtr memoryBlob, IntPtr originalAddress) { NetEventSource.Enter(null); // Return value. HTTP_VERB verb = HTTP_VERB.HttpVerbUnknown; - fixed (byte* pMemoryBlob = memoryBlob) + byte* pMemoryBlob = (byte*)memoryBlob; { HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; if ((int)request->Verb > (int)HTTP_VERB.HttpVerbUnparsed && (int)request->Verb < (int)HTTP_VERB.HttpVerbMaximum) @@ -936,14 +933,14 @@ internal static unsafe HTTP_VERB GetKnownVerb(byte[] memoryBlob, IntPtr original return verb; } - internal static unsafe IPEndPoint GetRemoteEndPoint(byte[] memoryBlob, IntPtr originalAddress) + internal static unsafe IPEndPoint GetRemoteEndPoint(IntPtr memoryBlob, IntPtr originalAddress) { if (NetEventSource.IsEnabled) NetEventSource.Enter(null); SocketAddress v4address = new SocketAddress(AddressFamily.InterNetwork, IPv4AddressSize); SocketAddress v6address = new SocketAddress(AddressFamily.InterNetworkV6, IPv6AddressSize); - fixed (byte* pMemoryBlob = memoryBlob) + byte* pMemoryBlob = (byte*)memoryBlob; { HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; IntPtr address = request->Address.pRemoteAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pRemoteAddress) : IntPtr.Zero; @@ -964,14 +961,14 @@ internal static unsafe IPEndPoint GetRemoteEndPoint(byte[] memoryBlob, IntPtr or return endpoint; } - internal static unsafe IPEndPoint GetLocalEndPoint(byte[] memoryBlob, IntPtr originalAddress) + internal static unsafe IPEndPoint GetLocalEndPoint(IntPtr memoryBlob, IntPtr originalAddress) { if (NetEventSource.IsEnabled) NetEventSource.Enter(null); SocketAddress v4address = new SocketAddress(AddressFamily.InterNetwork, IPv4AddressSize); SocketAddress v6address = new SocketAddress(AddressFamily.InterNetworkV6, IPv6AddressSize); - fixed (byte* pMemoryBlob = memoryBlob) + byte* pMemoryBlob = (byte*)memoryBlob; { HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; IntPtr address = request->Address.pLocalAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pLocalAddress) : IntPtr.Zero; diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs index b2d2d7258f2c..138db12aff03 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs @@ -38,7 +38,7 @@ internal AsyncRequestContext(ThreadPoolBoundHandle boundHandle, ListenerAsyncRes private Interop.HttpApi.HTTP_REQUEST* Allocate(ThreadPoolBoundHandle boundHandle, uint size) { - uint newSize = size != 0 ? size : RequestBuffer == null ? 4096 : Size; + uint newSize = size != 0 ? size : RequestBuffer == IntPtr.Zero ? 4096 : Size; if (_nativeOverlapped != null) { #if DEBUG @@ -57,7 +57,7 @@ internal AsyncRequestContext(ThreadPoolBoundHandle boundHandle, ListenerAsyncRes _boundHandle = boundHandle; _nativeOverlapped = boundHandle.AllocateNativeOverlapped(ListenerAsyncResult.IOCallback, state: _result, pinData: RequestBuffer); - return (Interop.HttpApi.HTTP_REQUEST*)Marshal.UnsafeAddrOfPinnedArrayElement(RequestBuffer, 0); + return (Interop.HttpApi.HTTP_REQUEST*)RequestBuffer.ToPointer(); } internal void Reset(ThreadPoolBoundHandle boundHandle, ulong requestId, uint size) diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/HttpListenerRequest.Windows.cs b/src/System.Net.HttpListener/src/System/Net/Windows/HttpListenerRequest.Windows.cs index b947bf5b304c..177f8222b828 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/HttpListenerRequest.Windows.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/HttpListenerRequest.Windows.cs @@ -109,7 +109,7 @@ internal HttpListenerRequest(HttpListenerContext httpContext, RequestContextBase // Note: RequestBuffer may get moved in memory. If you dereference a pointer from inside the RequestBuffer, // you must use 'OriginalBlobAddress' below to adjust the location of the pointer to match the location of // RequestBuffer. - internal byte[] RequestBuffer + internal IntPtr RequestBuffer { get { diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs index 6c73b70fa5ae..2dfe757ae1c3 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; +using System.Runtime.InteropServices; namespace System.Net { @@ -10,11 +11,12 @@ internal abstract unsafe class RequestContextBase : IDisposable { private Interop.HttpApi.HTTP_REQUEST* _memoryBlob; private Interop.HttpApi.HTTP_REQUEST* _originalBlobAddress; - private byte[] _backingBuffer; + private IntPtr _backingBuffer = IntPtr.Zero; + private int _backingBufferLength = 0; // Must call this from derived class' constructors. protected void BaseConstruction(Interop.HttpApi.HTTP_REQUEST* requestBlob) - { + { if (requestBlob == null) { GC.SuppressFinalize(this); @@ -29,9 +31,9 @@ protected void BaseConstruction(Interop.HttpApi.HTTP_REQUEST* requestBlob) // before an object (HttpListenerRequest) which closes the RequestContext on demand is returned to the application. internal void ReleasePins() { - Debug.Assert(_memoryBlob != null || _backingBuffer == null, "RequestContextBase::ReleasePins()|ReleasePins() called twice."); + Debug.Assert(_memoryBlob != null || _backingBuffer == IntPtr.Zero, "RequestContextBase::ReleasePins()|ReleasePins() called twice."); _originalBlobAddress = _memoryBlob; - UnsetBlob(); + UnsetBlob(); OnReleasePins(); } @@ -59,12 +61,12 @@ internal Interop.HttpApi.HTTP_REQUEST* RequestBlob { get { - Debug.Assert(_memoryBlob != null || _backingBuffer == null, "RequestContextBase::Dispose()|RequestBlob requested after ReleasePins()."); + Debug.Assert(_memoryBlob != null || _backingBuffer == IntPtr.Zero, "RequestContextBase::Dispose()|RequestBlob requested after ReleasePins()."); return _memoryBlob; } } - internal byte[] RequestBuffer + internal IntPtr RequestBuffer { get { @@ -76,7 +78,7 @@ internal uint Size { get { - return (uint)_backingBuffer.Length; + return (uint)_backingBufferLength; } } @@ -91,7 +93,7 @@ internal IntPtr OriginalBlobAddress protected void SetBlob(Interop.HttpApi.HTTP_REQUEST* requestBlob) { - Debug.Assert(_memoryBlob != null || _backingBuffer == null, "RequestContextBase::Dispose()|SetBlob() called after ReleasePins()."); + Debug.Assert(_memoryBlob != null || _backingBuffer == IntPtr.Zero, "RequestContextBase::Dispose()|SetBlob() called after ReleasePins()."); if (requestBlob == null) { UnsetBlob(); @@ -116,7 +118,19 @@ protected void UnsetBlob() protected void SetBuffer(int size) { - _backingBuffer = size == 0 ? null : new byte[size]; + if(_backingBuffer != IntPtr.Zero) + { + Marshal.FreeHGlobal(_backingBuffer); + } + + _backingBuffer = size == 0 ? IntPtr.Zero : Marshal.AllocHGlobal(size); + _backingBufferLength = size; + + for(int i = 0; i < size; ++i) + { + Marshal.WriteByte(_backingBuffer + i, 0); + } + Debug.Assert(size == 0 || _backingBuffer.ToInt64() % 8 == 0); } } } diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs index f7ba1e4f1b54..65abc946ec25 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs @@ -20,19 +20,19 @@ internal SyncRequestContext(int size) { if (_pinnedHandle.IsAllocated) { - if (RequestBuffer.Length == size) + if (Size == size) { return RequestBlob; } _pinnedHandle.Free(); } SetBuffer(size); - if (RequestBuffer == null) + if (RequestBuffer == IntPtr.Zero) { return null; } _pinnedHandle = GCHandle.Alloc(RequestBuffer, GCHandleType.Pinned); - return (Interop.HttpApi.HTTP_REQUEST*)Marshal.UnsafeAddrOfPinnedArrayElement(RequestBuffer, 0); + return (Interop.HttpApi.HTTP_REQUEST*)RequestBuffer.ToPointer(); } internal void Reset(int size) From d0fd8b82ba296d40e4819146897078aa411ea41e Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Mon, 27 Nov 2017 09:15:31 -0800 Subject: [PATCH 2/7] Merge upstream changes --- .../src/System/Net/Windows/RequestContextBase.cs | 6 +++++- .../src/System/Net/Windows/SyncRequestContext.cs | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs index 2dfe757ae1c3..3cfdb312b290 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs @@ -55,6 +55,10 @@ protected virtual void Dispose(bool disposing) { } ~RequestContextBase() { Dispose(false); + if (_backingBuffer != IntPtr.Zero) + { + Marshal.FreeHGlobal(_backingBuffer); + } } internal Interop.HttpApi.HTTP_REQUEST* RequestBlob @@ -130,7 +134,7 @@ protected void SetBuffer(int size) { Marshal.WriteByte(_backingBuffer + i, 0); } - Debug.Assert(size == 0 || _backingBuffer.ToInt64() % 8 == 0); + //Debug.Assert(size == 0 || _backingBuffer.ToInt64() % 8 == 0); } } } diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs index 65abc946ec25..77fb1c1c856d 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs @@ -24,7 +24,7 @@ internal SyncRequestContext(int size) { return RequestBlob; } - _pinnedHandle.Free(); + //_pinnedHandle.Free(); } SetBuffer(size); if (RequestBuffer == IntPtr.Zero) @@ -44,7 +44,7 @@ protected override void OnReleasePins() { if (_pinnedHandle.IsAllocated) { - _pinnedHandle.Free(); + //_pinnedHandle.Free(); } } From 7305eba1927b817fa1fc7fca93d73ff116893161 Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Tue, 28 Nov 2017 09:38:48 -0800 Subject: [PATCH 3/7] Fix leaking memory, clean up unnecessary pinning. --- .../Windows/HttpApi/Interop.HttpApi.cs | 141 +++++++++--------- .../System/Net/Windows/AsyncRequestContext.cs | 5 +- .../System/Net/Windows/RequestContextBase.cs | 2 +- .../System/Net/Windows/SyncRequestContext.cs | 43 +----- 4 files changed, 79 insertions(+), 112 deletions(-) diff --git a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs index c0f056c18f0c..459d647debcf 100644 --- a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs +++ b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs @@ -800,52 +800,51 @@ internal static unsafe WebHeaderCollection GetHeaders(IntPtr memoryBlob, IntPtr // Return value. WebHeaderCollection headerCollection = new WebHeaderCollection(); byte* pMemoryBlob = (byte*)memoryBlob; - { - HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; - long fixup = pMemoryBlob - (byte*)originalAddress; - int index; + + HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; + long fixup = pMemoryBlob - (byte*)originalAddress; + int index; - // unknown headers - if (request->Headers.UnknownHeaderCount != 0) + // unknown headers + if (request->Headers.UnknownHeaderCount != 0) + { + HTTP_UNKNOWN_HEADER* pUnknownHeader = (HTTP_UNKNOWN_HEADER*)(fixup + (byte*)request->Headers.pUnknownHeaders); + for (index = 0; index < request->Headers.UnknownHeaderCount; index++) { - HTTP_UNKNOWN_HEADER* pUnknownHeader = (HTTP_UNKNOWN_HEADER*)(fixup + (byte*)request->Headers.pUnknownHeaders); - for (index = 0; index < request->Headers.UnknownHeaderCount; index++) + // For unknown headers, when header value is empty, RawValueLength will be 0 and + // pRawValue will be null. + if (pUnknownHeader->pName != null && pUnknownHeader->NameLength > 0) { - // For unknown headers, when header value is empty, RawValueLength will be 0 and - // pRawValue will be null. - if (pUnknownHeader->pName != null && pUnknownHeader->NameLength > 0) + string headerName = new string(pUnknownHeader->pName + fixup, 0, pUnknownHeader->NameLength); + string headerValue; + if (pUnknownHeader->pRawValue != null && pUnknownHeader->RawValueLength > 0) { - string headerName = new string(pUnknownHeader->pName + fixup, 0, pUnknownHeader->NameLength); - string headerValue; - if (pUnknownHeader->pRawValue != null && pUnknownHeader->RawValueLength > 0) - { - headerValue = new string(pUnknownHeader->pRawValue + fixup, 0, pUnknownHeader->RawValueLength); - } - else - { - headerValue = string.Empty; - } - headerCollection.Add(headerName, headerValue); + headerValue = new string(pUnknownHeader->pRawValue + fixup, 0, pUnknownHeader->RawValueLength); } - pUnknownHeader++; + else + { + headerValue = string.Empty; + } + headerCollection.Add(headerName, headerValue); } + pUnknownHeader++; } + } - // known headers - HTTP_KNOWN_HEADER* pKnownHeader = &request->Headers.KnownHeaders; - for (index = 0; index < HttpHeaderRequestMaximum; index++) + // known headers + HTTP_KNOWN_HEADER* pKnownHeader = &request->Headers.KnownHeaders; + for (index = 0; index < HttpHeaderRequestMaximum; index++) + { + // For known headers, when header value is empty, RawValueLength will be 0 and + // pRawValue will point to empty string ("\0") + if (pKnownHeader->pRawValue != null) { - // For known headers, when header value is empty, RawValueLength will be 0 and - // pRawValue will point to empty string ("\0") - if (pKnownHeader->pRawValue != null) - { - string headerValue = new string(pKnownHeader->pRawValue + fixup, 0, pKnownHeader->RawValueLength); - headerCollection.Add(HTTP_REQUEST_HEADER_ID.ToString(index), headerValue); - } - pKnownHeader++; + string headerValue = new string(pKnownHeader->pRawValue + fixup, 0, pKnownHeader->RawValueLength); + headerCollection.Add(HTTP_REQUEST_HEADER_ID.ToString(index), headerValue); } + pKnownHeader++; } - + NetEventSource.Exit(null); return headerCollection; } @@ -861,52 +860,51 @@ internal static unsafe uint GetChunks(IntPtr memoryBlob, IntPtr originalAddress, // Return value. uint dataRead = 0; byte* pMemoryBlob = (byte*)memoryBlob; + + HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; + long fixup = pMemoryBlob - (byte*)originalAddress; + + if (request->EntityChunkCount > 0 && dataChunkIndex < request->EntityChunkCount && dataChunkIndex != -1) { - HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; - long fixup = pMemoryBlob - (byte*)originalAddress; + HTTP_DATA_CHUNK* pDataChunk = (HTTP_DATA_CHUNK*)(fixup + (byte*)&request->pEntityChunks[dataChunkIndex]); - if (request->EntityChunkCount > 0 && dataChunkIndex < request->EntityChunkCount && dataChunkIndex != -1) + fixed (byte* pReadBuffer = buffer) { - HTTP_DATA_CHUNK* pDataChunk = (HTTP_DATA_CHUNK*)(fixup + (byte*)&request->pEntityChunks[dataChunkIndex]); + byte* pTo = &pReadBuffer[offset]; - fixed (byte* pReadBuffer = buffer) + while (dataChunkIndex < request->EntityChunkCount && dataRead < size) { - byte* pTo = &pReadBuffer[offset]; - - while (dataChunkIndex < request->EntityChunkCount && dataRead < size) + if (dataChunkOffset >= pDataChunk->BufferLength) + { + dataChunkOffset = 0; + dataChunkIndex++; + pDataChunk++; + } + else { - if (dataChunkOffset >= pDataChunk->BufferLength) + byte* pFrom = pDataChunk->pBuffer + dataChunkOffset + fixup; + + uint bytesToRead = pDataChunk->BufferLength - (uint)dataChunkOffset; + if (bytesToRead > (uint)size) { - dataChunkOffset = 0; - dataChunkIndex++; - pDataChunk++; + bytesToRead = (uint)size; } - else + for (uint i = 0; i < bytesToRead; i++) { - byte* pFrom = pDataChunk->pBuffer + dataChunkOffset + fixup; - - uint bytesToRead = pDataChunk->BufferLength - (uint)dataChunkOffset; - if (bytesToRead > (uint)size) - { - bytesToRead = (uint)size; - } - for (uint i = 0; i < bytesToRead; i++) - { - *(pTo++) = *(pFrom++); - } - dataRead += bytesToRead; - dataChunkOffset += bytesToRead; + *(pTo++) = *(pFrom++); } + dataRead += bytesToRead; + dataChunkOffset += bytesToRead; } } } - //we're finished. - if (dataChunkIndex == request->EntityChunkCount) - { - dataChunkIndex = -1; - } } - + //we're finished. + if (dataChunkIndex == request->EntityChunkCount) + { + dataChunkIndex = -1; + } + if (NetEventSource.IsEnabled) { NetEventSource.Exit(null); @@ -969,11 +967,10 @@ internal static unsafe IPEndPoint GetLocalEndPoint(IntPtr memoryBlob, IntPtr ori SocketAddress v6address = new SocketAddress(AddressFamily.InterNetworkV6, IPv6AddressSize); byte* pMemoryBlob = (byte*)memoryBlob; - { - HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; - IntPtr address = request->Address.pLocalAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pLocalAddress) : IntPtr.Zero; - CopyOutAddress(address, ref v4address, ref v6address); - } + + HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; + IntPtr address = request->Address.pLocalAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pLocalAddress) : IntPtr.Zero; + CopyOutAddress(address, ref v4address, ref v6address); IPEndPoint endpoint = null; if (v4address != null) diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs index 138db12aff03..33c3c16a0648 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs @@ -56,7 +56,6 @@ internal AsyncRequestContext(ThreadPoolBoundHandle boundHandle, ListenerAsyncRes SetBuffer(checked((int)newSize)); _boundHandle = boundHandle; _nativeOverlapped = boundHandle.AllocateNativeOverlapped(ListenerAsyncResult.IOCallback, state: _result, pinData: RequestBuffer); - return (Interop.HttpApi.HTTP_REQUEST*)RequestBuffer.ToPointer(); } @@ -84,7 +83,7 @@ protected override void Dispose(bool disposing) { if (_nativeOverlapped != null) { - Debug.Assert(!disposing, "AsyncRequestContext::Dispose()|Must call ReleasePins() before calling Dispose()."); + //Debug.Assert(!disposing, "AsyncRequestContext::Dispose()|Must call ReleasePins() before calling Dispose()."); if (!Environment.HasShutdownStarted || disposing) { #if DEBUG @@ -102,7 +101,7 @@ internal NativeOverlapped* NativeOverlapped get { #if DEBUG - Debug.Assert(Interlocked.Increment(ref _nativeOverlappedUsed) == 1, "NativeOverlapped reused."); + //Debug.Assert(Interlocked.Increment(ref _nativeOverlappedUsed) == 1, "NativeOverlapped reused."); #endif return _nativeOverlapped; diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs index 3cfdb312b290..675414bd6beb 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs @@ -130,11 +130,11 @@ protected void SetBuffer(int size) _backingBuffer = size == 0 ? IntPtr.Zero : Marshal.AllocHGlobal(size); _backingBufferLength = size; + // Zero out the contents of the buffer. for(int i = 0; i < size; ++i) { Marshal.WriteByte(_backingBuffer + i, 0); } - //Debug.Assert(size == 0 || _backingBuffer.ToInt64() % 8 == 0); } } } diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs index 77fb1c1c856d..7aceb4bbe5ed 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs @@ -9,30 +9,20 @@ namespace System.Net { internal unsafe class SyncRequestContext : RequestContextBase { - private GCHandle _pinnedHandle; - internal SyncRequestContext(int size) { BaseConstruction(Allocate(size)); } - private Interop.HttpApi.HTTP_REQUEST* Allocate(int size) + private Interop.HttpApi.HTTP_REQUEST* Allocate(int newSize) { - if (_pinnedHandle.IsAllocated) - { - if (Size == size) - { - return RequestBlob; - } - //_pinnedHandle.Free(); - } - SetBuffer(size); - if (RequestBuffer == IntPtr.Zero) + if (Size > 0 && Size == newSize) { - return null; + return RequestBlob; } - _pinnedHandle = GCHandle.Alloc(RequestBuffer, GCHandleType.Pinned); - return (Interop.HttpApi.HTTP_REQUEST*)RequestBuffer.ToPointer(); + SetBuffer(newSize); + + return RequestBuffer == IntPtr.Zero ? null : (Interop.HttpApi.HTTP_REQUEST*)RequestBuffer.ToPointer(); } internal void Reset(int size) @@ -40,25 +30,6 @@ internal void Reset(int size) SetBlob(Allocate(size)); } - protected override void OnReleasePins() - { - if (_pinnedHandle.IsAllocated) - { - //_pinnedHandle.Free(); - } - } - - protected override void Dispose(bool disposing) - { - if (_pinnedHandle.IsAllocated) - { - Debug.Assert(!disposing, "AsyncRequestContext::Dispose()|Must call ReleasePins() before calling Dispose()."); - if (!Environment.HasShutdownStarted || disposing) - { - _pinnedHandle.Free(); - } - } - base.Dispose(disposing); - } + protected override void OnReleasePins() { } } } From fd0667f4f0324d4628f82a39703bef1ee5891ac9 Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Tue, 28 Nov 2017 11:36:19 -0800 Subject: [PATCH 4/7] Formatting fixes. --- .../Windows/HttpApi/Interop.HttpApi.cs | 29 +++++++------------ .../System/Net/Windows/AsyncRequestContext.cs | 5 ++-- .../System/Net/Windows/RequestContextBase.cs | 4 +-- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs index 459d647debcf..51dcb3e705ce 100644 --- a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs +++ b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs @@ -799,8 +799,7 @@ internal static unsafe WebHeaderCollection GetHeaders(IntPtr memoryBlob, IntPtr // Return value. WebHeaderCollection headerCollection = new WebHeaderCollection(); - byte* pMemoryBlob = (byte*)memoryBlob; - + byte* pMemoryBlob = (byte*)memoryBlob; HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; long fixup = pMemoryBlob - (byte*)originalAddress; int index; @@ -859,8 +858,7 @@ internal static unsafe uint GetChunks(IntPtr memoryBlob, IntPtr originalAddress, // Return value. uint dataRead = 0; - byte* pMemoryBlob = (byte*)memoryBlob; - + byte* pMemoryBlob = (byte*)memoryBlob; HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; long fixup = pMemoryBlob - (byte*)originalAddress; @@ -918,13 +916,11 @@ internal static unsafe HTTP_VERB GetKnownVerb(IntPtr memoryBlob, IntPtr original // Return value. HTTP_VERB verb = HTTP_VERB.HttpVerbUnknown; - byte* pMemoryBlob = (byte*)memoryBlob; + + HTTP_REQUEST* request = (HTTP_REQUEST*)memoryBlob.ToPointer(); + if ((int)request->Verb > (int)HTTP_VERB.HttpVerbUnparsed && (int)request->Verb < (int)HTTP_VERB.HttpVerbMaximum) { - HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; - if ((int)request->Verb > (int)HTTP_VERB.HttpVerbUnparsed && (int)request->Verb < (int)HTTP_VERB.HttpVerbMaximum) - { - verb = request->Verb; - } + verb = request->Verb; } NetEventSource.Exit(null); @@ -938,12 +934,10 @@ internal static unsafe IPEndPoint GetRemoteEndPoint(IntPtr memoryBlob, IntPtr or SocketAddress v4address = new SocketAddress(AddressFamily.InterNetwork, IPv4AddressSize); SocketAddress v6address = new SocketAddress(AddressFamily.InterNetworkV6, IPv6AddressSize); - byte* pMemoryBlob = (byte*)memoryBlob; - { - HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; - IntPtr address = request->Address.pRemoteAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pRemoteAddress) : IntPtr.Zero; - CopyOutAddress(address, ref v4address, ref v6address); - } + byte* pMemoryBlob = (byte*)memoryBlob; + HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; + IntPtr address = request->Address.pRemoteAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pRemoteAddress) : IntPtr.Zero; + CopyOutAddress(address, ref v4address, ref v6address); IPEndPoint endpoint = null; if (v4address != null) @@ -966,8 +960,7 @@ internal static unsafe IPEndPoint GetLocalEndPoint(IntPtr memoryBlob, IntPtr ori SocketAddress v4address = new SocketAddress(AddressFamily.InterNetwork, IPv4AddressSize); SocketAddress v6address = new SocketAddress(AddressFamily.InterNetworkV6, IPv6AddressSize); - byte* pMemoryBlob = (byte*)memoryBlob; - + byte* pMemoryBlob = (byte*)memoryBlob; HTTP_REQUEST* request = (HTTP_REQUEST*)pMemoryBlob; IntPtr address = request->Address.pLocalAddress != null ? (IntPtr)(pMemoryBlob - (byte*)originalAddress + (byte*)request->Address.pLocalAddress) : IntPtr.Zero; CopyOutAddress(address, ref v4address, ref v6address); diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs index 33c3c16a0648..138db12aff03 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/AsyncRequestContext.cs @@ -56,6 +56,7 @@ internal AsyncRequestContext(ThreadPoolBoundHandle boundHandle, ListenerAsyncRes SetBuffer(checked((int)newSize)); _boundHandle = boundHandle; _nativeOverlapped = boundHandle.AllocateNativeOverlapped(ListenerAsyncResult.IOCallback, state: _result, pinData: RequestBuffer); + return (Interop.HttpApi.HTTP_REQUEST*)RequestBuffer.ToPointer(); } @@ -83,7 +84,7 @@ protected override void Dispose(bool disposing) { if (_nativeOverlapped != null) { - //Debug.Assert(!disposing, "AsyncRequestContext::Dispose()|Must call ReleasePins() before calling Dispose()."); + Debug.Assert(!disposing, "AsyncRequestContext::Dispose()|Must call ReleasePins() before calling Dispose()."); if (!Environment.HasShutdownStarted || disposing) { #if DEBUG @@ -101,7 +102,7 @@ internal NativeOverlapped* NativeOverlapped get { #if DEBUG - //Debug.Assert(Interlocked.Increment(ref _nativeOverlappedUsed) == 1, "NativeOverlapped reused."); + Debug.Assert(Interlocked.Increment(ref _nativeOverlappedUsed) == 1, "NativeOverlapped reused."); #endif return _nativeOverlapped; diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs index 675414bd6beb..4d0123de230e 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs @@ -16,7 +16,7 @@ internal abstract unsafe class RequestContextBase : IDisposable // Must call this from derived class' constructors. protected void BaseConstruction(Interop.HttpApi.HTTP_REQUEST* requestBlob) - { + { if (requestBlob == null) { GC.SuppressFinalize(this); @@ -33,7 +33,7 @@ internal void ReleasePins() { Debug.Assert(_memoryBlob != null || _backingBuffer == IntPtr.Zero, "RequestContextBase::ReleasePins()|ReleasePins() called twice."); _originalBlobAddress = _memoryBlob; - UnsetBlob(); + UnsetBlob(); OnReleasePins(); } From 3ae9b2647137daf1c09d973cce8416ca3580bb75 Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Tue, 28 Nov 2017 13:35:09 -0800 Subject: [PATCH 5/7] RequestContextBase cleanup. --- .../src/System/Net/Windows/RequestContextBase.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs index 4d0123de230e..185cb28aeef9 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs @@ -50,17 +50,20 @@ public void Dispose() Dispose(true); } - protected virtual void Dispose(bool disposing) { } - - ~RequestContextBase() + protected virtual void Dispose(bool disposing) { - Dispose(false); if (_backingBuffer != IntPtr.Zero) { Marshal.FreeHGlobal(_backingBuffer); + _backingBuffer = IntPtr.Zero; } } + ~RequestContextBase() + { + Dispose(false); + } + internal Interop.HttpApi.HTTP_REQUEST* RequestBlob { get From 6c21edb869bb0d6cba8fc5fdefad583a4d00b35c Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Wed, 29 Nov 2017 10:02:55 -0800 Subject: [PATCH 6/7] Address review comments. --- src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs | 1 - .../src/System/Net/Windows/RequestContextBase.cs | 7 ++----- .../src/System/Net/Windows/SyncRequestContext.cs | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs index 51dcb3e705ce..679d5828855d 100644 --- a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs +++ b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs @@ -848,7 +848,6 @@ internal static unsafe WebHeaderCollection GetHeaders(IntPtr memoryBlob, IntPtr return headerCollection; } - internal static unsafe uint GetChunks(IntPtr memoryBlob, IntPtr originalAddress, ref int dataChunkIndex, ref uint dataChunkOffset, byte[] buffer, int offset, int size) { if (NetEventSource.IsEnabled) diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs index 185cb28aeef9..6f7127931846 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/RequestContextBase.cs @@ -125,7 +125,7 @@ protected void UnsetBlob() protected void SetBuffer(int size) { - if(_backingBuffer != IntPtr.Zero) + if (_backingBuffer != IntPtr.Zero) { Marshal.FreeHGlobal(_backingBuffer); } @@ -134,10 +134,7 @@ protected void SetBuffer(int size) _backingBufferLength = size; // Zero out the contents of the buffer. - for(int i = 0; i < size; ++i) - { - Marshal.WriteByte(_backingBuffer + i, 0); - } + new Span(_backingBuffer.ToPointer(), size).Fill(0); } } } diff --git a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs index 7aceb4bbe5ed..b140ab1564f0 100644 --- a/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs +++ b/src/System.Net.HttpListener/src/System/Net/Windows/SyncRequestContext.cs @@ -18,7 +18,7 @@ internal SyncRequestContext(int size) { if (Size > 0 && Size == newSize) { - return RequestBlob; + return RequestBlob; } SetBuffer(newSize); From 3f90e6ff6a0520597c3376dd4537ff802c37103d Mon Sep 17 00:00:00 2001 From: Max Kerr Date: Wed, 29 Nov 2017 14:05:41 -0800 Subject: [PATCH 7/7] Whitespace. --- src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs index 679d5828855d..38d8491d5aab 100644 --- a/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs +++ b/src/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs @@ -843,7 +843,7 @@ internal static unsafe WebHeaderCollection GetHeaders(IntPtr memoryBlob, IntPtr } pKnownHeader++; } - + NetEventSource.Exit(null); return headerCollection; }