Adding MemoryHandle AddOffset to fix Memory.Retain impl (dotnet/corefx#24323)

* Adding MemoryHandle AddOffset to fix Memory.Retain impl

* Adding AddOffset to System.Runtime ref.

* Make the AddOffset API internal.

Commit migrated from https://github.com/dotnet/corefx/commit/b9676be62166e90a07763f070330a9ffc8ef6cc7

diff --git a/src/libraries/System.Memory/ref/System.Memory.cs b/src/libraries/System.Memory/ref/System.Memory.cs
index 04b521e..b38abb4 100644 (file)
--- a/src/libraries/System.Memory/ref/System.Memory.cs
+++ b/src/libraries/System.Memory/ref/System.Memory.cs
@@ -178,7 +178,8 @@ namespace System.Buffers
     {
         public MemoryHandle(IRetainable owner, void* pinnedPointer = null,  System.Runtime.InteropServices.GCHandle handle = default(System.Runtime.InteropServices.GCHandle))  { throw null; }
         public void* PinnedPointer { get { throw null; } }
-        public void Dispose()  { throw null; }
+        internal void AddOffset(int offset) { throw null; }
+        public void Dispose() { throw null; }
     }
 
     public interface IRetainable 

diff --git a/src/libraries/System.Memory/src/System/Buffers/MemoryHandle.cs b/src/libraries/System.Memory/src/System/Buffers/MemoryHandle.cs
index 315d070..68c4b48 100644 (file)
--- a/src/libraries/System.Memory/src/System/Buffers/MemoryHandle.cs
+++ b/src/libraries/System.Memory/src/System/Buffers/MemoryHandle.cs
@@ -35,6 +35,24 @@ namespace System.Buffers
         public void* PinnedPointer => _pointer;
 
         /// <summary>
+        /// Adds an offset to the pinned pointer.
+        /// </summary>
+        /// <exception cref="System.ArgumentNullException">
+        /// Throw when pinned pointer is null.
+        /// </exception>
+        internal void AddOffset(int offset)
+        {
+            if (_pointer == null)
+            {
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.pointer);
+            }
+            else
+            {
+                _pointer = (void*)((byte*)_pointer + offset);
+            }
+        }
+
+        /// <summary>
         /// Frees the pinned handle and releases IRetainable.
         /// </summary>
         public void Dispose()

diff --git a/src/libraries/System.Memory/src/System/Memory.cs b/src/libraries/System.Memory/src/System/Memory.cs
index d81e05f..b311bb8 100644 (file)
--- a/src/libraries/System.Memory/src/System/Memory.cs
+++ b/src/libraries/System.Memory/src/System/Memory.cs
@@ -193,6 +193,7 @@ namespace System
                 if (_index < 0)
                 {
                     memoryHandle = ((OwnedMemory<T>)_arrayOrOwnedMemory).Pin();
+                    memoryHandle.AddOffset((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
                 }
                 else
                 {

diff --git a/src/libraries/System.Memory/src/System/ReadOnlyMemory.cs b/src/libraries/System.Memory/src/System/ReadOnlyMemory.cs
index c1764bf..f95dba1 100644 (file)
--- a/src/libraries/System.Memory/src/System/ReadOnlyMemory.cs
+++ b/src/libraries/System.Memory/src/System/ReadOnlyMemory.cs
@@ -178,6 +178,7 @@ namespace System
                 if (_index < 0)
                 {
                     memoryHandle = ((OwnedMemory<T>)_arrayOrOwnedMemory).Pin();
+                    memoryHandle.AddOffset((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
                 }
                 else
                 {

diff --git a/src/libraries/System.Memory/src/System/ThrowHelper.cs b/src/libraries/System.Memory/src/System/ThrowHelper.cs
index 18de642..cce75a8 100644 (file)
--- a/src/libraries/System.Memory/src/System/ThrowHelper.cs
+++ b/src/libraries/System.Memory/src/System/ThrowHelper.cs
@@ -63,6 +63,7 @@ namespace System
         start,
         text,
         obj,
-        ownedMemory
+        ownedMemory,
+        pointer
     }
 }

diff --git a/src/libraries/System.Memory/tests/Memory/Retain.cs b/src/libraries/System.Memory/tests/Memory/Retain.cs
index d64a9d0..544d8a5 100644 (file)
--- a/src/libraries/System.Memory/tests/Memory/Retain.cs
+++ b/src/libraries/System.Memory/tests/Memory/Retain.cs
@@ -46,6 +46,34 @@ namespace System.MemoryTests
         }
 
         [Fact]
+        public static void MemoryRetainWithPinningAndSlice()
+        {
+            int[] array = { 1, 2, 3, 4, 5 };
+            Memory<int> memory = array;
+            memory = memory.Slice(1);
+            MemoryHandle handle = memory.Retain(pin: true);
+            Span<int> span = memory.Span;
+            unsafe
+            {
+                int* pointer = (int*)handle.PinnedPointer;
+                Assert.True(pointer != null);
+
+                GC.Collect();
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], pointer[i]);
+                }
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], span[i]);
+                }
+            }
+            handle.Dispose();
+        }
+
+        [Fact]
         public static void OwnedMemoryRetainWithoutPinning()
         {
             int[] array = { 1, 2, 3, 4, 5 };
@@ -81,5 +109,33 @@ namespace System.MemoryTests
             }
             handle.Dispose();
         }
+
+        [Fact]
+        public static void OwnedMemoryRetainWithPinningAndSlice()
+        {
+            int[] array = { 1, 2, 3, 4, 5 };
+            OwnedMemory<int> owner = new CustomMemoryForTest<int>(array);
+            Memory<int> memory = owner.Memory.Slice(1);
+            MemoryHandle handle = memory.Retain(pin: true);
+            Span<int> span = memory.Span;
+            unsafe
+            {
+                int* pointer = (int*)handle.PinnedPointer;
+                Assert.True(pointer != null);
+
+                GC.Collect();
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], pointer[i]);
+                }
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], span[i]);
+                }
+            }
+            handle.Dispose();
+        }
     }
 }

diff --git a/src/libraries/System.Memory/tests/ReadOnlyMemory/Retain.cs b/src/libraries/System.Memory/tests/ReadOnlyMemory/Retain.cs
index 437b6f3..9838bc7 100644 (file)
--- a/src/libraries/System.Memory/tests/ReadOnlyMemory/Retain.cs
+++ b/src/libraries/System.Memory/tests/ReadOnlyMemory/Retain.cs
@@ -46,6 +46,34 @@ namespace System.MemoryTests
         }
 
         [Fact]
+        public static void MemoryRetainWithPinningAndSlice()
+        {
+            int[] array = { 1, 2, 3, 4, 5 };
+            ReadOnlyMemory<int> memory = array;
+            memory = memory.Slice(1);
+            MemoryHandle handle = memory.Retain(pin: true);
+            ReadOnlySpan<int> span = memory.Span;
+            unsafe
+            {
+                int* pointer = (int*)handle.PinnedPointer;
+                Assert.True(pointer != null);
+
+                GC.Collect();
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], pointer[i]);
+                }
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], span[i]);
+                }
+            }
+            handle.Dispose();
+        }
+
+        [Fact]
         public static void OwnedMemoryRetainWithoutPinning()
         {
             int[] array = { 1, 2, 3, 4, 5 };
@@ -81,5 +109,33 @@ namespace System.MemoryTests
             }
             handle.Dispose();
         }
+
+        [Fact]
+        public static void OwnedMemoryRetainWithPinningAndSlice()
+        {
+            int[] array = { 1, 2, 3, 4, 5 };
+            OwnedMemory<int> owner = new CustomMemoryForTest<int>(array);
+            ReadOnlyMemory<int> memory = owner.Memory.Slice(1);
+            MemoryHandle handle = memory.Retain(pin: true);
+            ReadOnlySpan<int> span = memory.Span;
+            unsafe
+            {
+                int* pointer = (int*)handle.PinnedPointer;
+                Assert.True(pointer != null);
+
+                GC.Collect();
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], pointer[i]);
+                }
+
+                for (int i = 0; i < memory.Length; i++)
+                {
+                    Assert.Equal(array[i + 1], span[i]);
+                }
+            }
+            handle.Dispose();
+        }
     }
 }

diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs
index a8cf195..5442cfe 100644 (file)
--- a/src/libraries/System.Runtime/ref/System.Runtime.cs
+++ b/src/libraries/System.Runtime/ref/System.Runtime.cs
@@ -3841,7 +3841,8 @@ namespace System.Buffers
         public MemoryHandle(IRetainable owner, void* pinnedPointer = null,  System.Runtime.InteropServices.GCHandle handle = default(System.Runtime.InteropServices.GCHandle))  { throw null; }
         [System.CLSCompliantAttribute(false)]
         public void* PinnedPointer { get { throw null; } }
-        public void Dispose()  { throw null; }
+        internal void AddOffset(int offset) { throw null; }
+        public void Dispose() { throw null; }
     }
 
     public interface IRetainable