From 12a3816c122f0d86968922480bd2e53f42bde365 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 5 May 2016 15:00:58 -0700 Subject: [PATCH] Make some MemoryPoolBlock operations volatile - Making MemoryPoolBlock.End volatile and using Volatile.Write for MemoryPoolBlock.Next prevents a race that could cause threads consuming blocks to skip produced bytes. --- src/Microsoft.AspNetCore.Server.Kestrel/Http/SocketInput.cs | 2 +- .../Infrastructure/MemoryPoolBlock.cs | 6 +++--- .../Infrastructure/MemoryPoolIterator.cs | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/SocketInput.cs index 1588e83011..40532e17b4 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Http/SocketInput.cs @@ -105,7 +105,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http } else { - _tail.Next = _pinned; + Volatile.Write(ref _tail.Next, _pinned); _tail = _pinned; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolBlock.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolBlock.cs index 25cab8c4f4..1bcd60ecaa 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolBlock.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolBlock.cs @@ -57,14 +57,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure /// the Start is guaranteed to be equal to Array.Offset. The value of Start may be assigned anywhere between Data.Offset and /// Data.Offset + Data.Count, and must be equal to or less than End. /// - public int Start { get; set; } + public int Start; /// /// The End represents the offset into Array where the range of "active" bytes ends. At the point when the block is leased /// the End is guaranteed to be equal to Array.Offset. The value of Start may be assigned anywhere between Data.Offset and /// Data.Offset + Data.Count, and must be equal to or less than End. /// - public int End { get; set; } + public volatile int End; /// /// Reference to the next block of data when the overall "active" bytes spans multiple blocks. At the point when the block is @@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure /// working memory. The "active" memory is grown when bytes are copied in, End is increased, and Next is assigned. The "active" /// memory is shrunk when bytes are consumed, Start is increased, and blocks are returned to the pool. /// - public MemoryPoolBlock Next { get; set; } + public MemoryPoolBlock Next; ~MemoryPoolBlock() { diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIterator.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIterator.cs index 8d65c5c377..202d15191f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIterator.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIterator.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.Numerics; +using System.Threading; namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure { @@ -767,7 +768,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure { var nextBlock = pool.Lease(); block.End = blockIndex; - block.Next = nextBlock; + Volatile.Write(ref block.Next, nextBlock); block = nextBlock; blockIndex = block.Data.Offset; @@ -820,7 +821,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure { var nextBlock = pool.Lease(); block.End = blockIndex; - block.Next = nextBlock; + Volatile.Write(ref block.Next, nextBlock); block = nextBlock; blockIndex = block.Data.Offset;