Change OwnedMemory to MemoryManager and add an IMemoryOwner. (#17340)

* Change OwnedMemory to MemoryManager and add an IMemoryOwner.

* Fix comments.

* Reset start and length if TryGetMemoryManager returns false.

* Reset start and length if TryGetMemoryManager returns false [actually].

* Re-order MemoryHandle ctor parameters and rename TOwner to TManager.

* Fix formatting, remove unused code, and fix impl of Pin()

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

11 files changed, 266 insertions, 304 deletions

diff --git a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems
index 114358017..d5c432e12 100644
--- a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems
+++ b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems
@@ -46,9 +46,10 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\ArrayPool.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\ArrayPoolEventSource.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\ConfigurableArrayPool.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\IRetainable.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\IMemoryOwner.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\IPinnable.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\MemoryHandle.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\OwnedMemory.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\MemoryManager.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\TlsOverPerCoreLockedStacksArrayPool.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffers\Utilities.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Byte.cs" />
diff --git a/src/System.Private.CoreLib/shared/System/Buffers/IMemoryOwner.cs b/src/System.Private.CoreLib/shared/System/Buffers/IMemoryOwner.cs
new file mode 100644
index 000000000..44f16c582
--- /dev/null
+++ b/src/System.Private.CoreLib/shared/System/Buffers/IMemoryOwner.cs
@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System.Buffers
+{
+    /// <summary>
+    /// Owner of Memory<typeparamref name="T"/> that is responsible for disposing the underlying memory appropriately.
+    /// </summary>
+    public interface IMemoryOwner<T> : IDisposable
+    {
+        /// <summary>
+        /// Returns a Memory<typeparamref name="T"/>.
+        /// </summary>
+        Memory<T> Memory { get; }
+    }
+}
diff --git a/src/System.Private.CoreLib/shared/System/Buffers/IPinnable.cs b/src/System.Private.CoreLib/shared/System/Buffers/IPinnable.cs
new file mode 100644
index 000000000..623e716a0
--- /dev/null
+++ b/src/System.Private.CoreLib/shared/System/Buffers/IPinnable.cs
@@ -0,0 +1,25 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System.Buffers
+{
+    /// <summary>
+    /// Provides a mechanism for pinning and unpinning objects to prevent the GC from moving them.
+    /// </summary>
+    public interface IPinnable
+    {
+        /// <summary>
+        /// Call this method to indicate that the IPinnable object can not be moved by the garbage collector.
+        /// The address of the pinned object can be taken.
+        /// <param name="elementIndex">The offset to the element within the memory at which the returned <see cref="MemoryHandle"/> points to.</param>
+        /// </summary>
+        MemoryHandle Pin(int elementIndex);
+
+        /// <summary>
+        /// Call this method to indicate that the IPinnable object no longer needs to be pinned.
+        /// The garbage collector is free to move the object now.
+        /// </summary>
+        void Unpin();
+    }
+}
diff --git a/src/System.Private.CoreLib/shared/System/Buffers/IRetainable.cs b/src/System.Private.CoreLib/shared/System/Buffers/IRetainable.cs
deleted file mode 100644
index 6ac508859..000000000
--- a/src/System.Private.CoreLib/shared/System/Buffers/IRetainable.cs
+++ /dev/null
@@ -1,26 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System.Runtime;
-using System.Runtime.CompilerServices;
-
-namespace System.Buffers
-{
-    /// <summary>
-    /// Provides a mechanism for manual lifetime management.
-    /// </summary>
-    public interface IRetainable
-    {
-        /// <summary>
-        /// Call this method to indicate that the IRetainable object is in use.
-        /// Do not dispose until Release is called.
-        /// </summary>
-        void Retain();
-        /// <summary>
-        /// Call this method to indicate that the IRetainable object is no longer in use.
-        /// The object can now be disposed.
-        /// </summary>
-        bool Release();
-    }
-}
\ No newline at end of file
diff --git a/src/System.Private.CoreLib/shared/System/Buffers/MemoryHandle.cs b/src/System.Private.CoreLib/shared/System/Buffers/MemoryHandle.cs
index 754403862..231add1c0 100644
--- a/src/System.Private.CoreLib/shared/System/Buffers/MemoryHandle.cs
+++ b/src/System.Private.CoreLib/shared/System/Buffers/MemoryHandle.cs
@@ -12,53 +12,48 @@ namespace System.Buffers
     /// </summary>
     public unsafe struct MemoryHandle : IDisposable
     {
-        private IRetainable _retainable;
         private void* _pointer;
         private GCHandle _handle;
+        private IPinnable _pinnable;
 
         /// <summary>
         /// Creates a new memory handle for the memory.
         /// </summary>
-        /// <param name="retainable">reference to manually managed object</param>
-        /// <param name="pointer">pointer to memory, or null if a pointer was not provided when the handle was created</param>
+        /// <param name="pointer">pointer to memory</param>
+        /// <param name="pinnable">reference to manually managed object, or default if there is no memory manager</param>
         /// <param name="handle">handle used to pin array buffers</param>
         [CLSCompliant(false)]
-        public MemoryHandle(IRetainable retainable, void* pointer = null, GCHandle handle = default(GCHandle))
+        public MemoryHandle(void* pointer, GCHandle handle = default, IPinnable pinnable = default)
         {
-            _retainable = retainable;
             _pointer = pointer;
             _handle = handle;
+            _pinnable = pinnable;
         }
 
         /// <summary>
-        /// Returns the pointer to memory, or null if a pointer was not provided when the handle was created.
+        /// Returns the pointer to memory, where the memory is assumed to be pinned and hence the address won't change.
         /// </summary>
         [CLSCompliant(false)]
         public void* Pointer => _pointer;
 
         /// <summary>
-        /// Returns false if the pointer to memory is null.
+        /// Frees the pinned handle and releases IPinnable.
         /// </summary>
-        public bool HasPointer => _pointer != null;
-
-        /// <summary>
-        /// Frees the pinned handle and releases IRetainable.
-        /// </summary>
-       public void Dispose()
+        public void Dispose()
         {
             if (_handle.IsAllocated)
             {
                 _handle.Free();
             }
 
-            if (_retainable != null)
+            if (_pinnable != null)
             {
-                _retainable.Release();
-                _retainable = null;
+                _pinnable.Unpin();
+                _pinnable = null;
             }
 
             _pointer = null;
         }
 
     }
-}
\ No newline at end of file
+}
diff --git a/src/System.Private.CoreLib/shared/System/Buffers/MemoryManager.cs b/src/System.Private.CoreLib/shared/System/Buffers/MemoryManager.cs
new file mode 100644
index 000000000..cc47e02a2
--- /dev/null
+++ b/src/System.Private.CoreLib/shared/System/Buffers/MemoryManager.cs
@@ -0,0 +1,66 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime;
+using System.Runtime.CompilerServices;
+
+namespace System.Buffers
+{
+    /// <summary>
+    /// Manager of Memory<typeparamref name="T"/> that provides the implementation.
+    /// </summary>
+    public abstract class MemoryManager<T> : IMemoryOwner<T>, IPinnable
+    {
+        /// <summary>
+        /// The number of items in the Memory<typeparamref name="T"/>.
+        /// </summary>
+        public virtual int Length => GetSpan().Length;
+
+        /// <summary>
+        /// Returns a Memory<typeparamref name="T"/>.
+        /// </summary>
+        public virtual Memory<T> Memory => new Memory<T>(this, 0, Length);
+
+        /// <summary>
+        /// Returns a span wrapping the underlying memory.
+        /// </summary>
+        public abstract Span<T> GetSpan();
+
+        /// <summary>
+        /// Returns a handle to the memory that has been pinned and hence its address can be taken.
+        /// <param name="elementIndex">The offset to the element within the memory at which the returned <see cref="MemoryHandle"/> points to. (default = 0)</param>
+        /// </summary>
+        public abstract MemoryHandle Pin(int elementIndex = 0);
+
+        /// <summary>
+        /// Lets the garbage collector know that the object is free to be moved now.
+        /// </summary>
+        public abstract void Unpin();
+
+        /// <summary>
+        /// Returns an array segment.
+        /// <remarks>Returns the default array segment if not overriden.</remarks>
+        /// </summary>
+        protected internal virtual bool TryGetArray(out ArraySegment<T> segment)
+        {
+            segment = default;
+            return false;
+        }
+
+        /// <summary>
+        /// Implements IDisposable.
+        /// </summary>
+        void IDisposable.Dispose()
+        {
+            Dispose(disposing: true);
+            GC.SuppressFinalize(this);
+        }
+
+        /// <summary>
+        /// Clean up of any leftover managed and unmanaged resources.
+        /// </summary>
+        protected abstract void Dispose(bool disposing);
+
+    }
+}
diff --git a/src/System.Private.CoreLib/shared/System/Buffers/OwnedMemory.cs b/src/System.Private.CoreLib/shared/System/Buffers/OwnedMemory.cs
deleted file mode 100644
index 8acd5b224..000000000
--- a/src/System.Private.CoreLib/shared/System/Buffers/OwnedMemory.cs
+++ /dev/null
@@ -1,95 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System.Runtime;
-using System.Runtime.CompilerServices;
-
-namespace System.Buffers
-{
-    /// <summary>
-    /// Owner of Memory<typeparamref name="T"/> that provides appropriate lifetime management mechanisms for it.
-    /// </summary>
-    public abstract class OwnedMemory<T> : IDisposable, IRetainable
-    {
-        /// <summary>
-        /// The number of items in the Memory<typeparamref name="T"/>.
-        /// </summary>
-        public abstract int Length { get; }
-
-        /// <summary>
-        /// Returns a span wrapping the underlying memory.
-        /// </summary>
-        public abstract Span<T> Span { get; }
-
-        /// <summary>
-        /// Returns a Memory<typeparamref name="T"/> if the underlying memory has not been freed.
-        /// </summary>
-        /// <exception cref="System.ObjectDisposedException">
-        /// Thrown when the underlying memory has already been disposed.
-        /// </exception>
-        public Memory<T> Memory
-        {
-            get
-            {
-                if (IsDisposed)
-                {
-                    ThrowHelper.ThrowObjectDisposedException_MemoryDisposed();
-                }
-                return new Memory<T>(owner: this, 0, Length);
-            }
-        }
-
-        /// <summary>
-        /// Returns a handle for the array that has been pinned and hence its address can be taken
-        /// </summary>
-        public abstract MemoryHandle Pin(int byteOffset = 0);
-
-        /// <summary>
-        /// Returns an array segment.
-        /// </summary>
-        protected internal abstract bool TryGetArray(out ArraySegment<T> segment);
-
-        /// <summary>
-        /// Implements IDisposable.
-        /// </summary>
-        /// <exception cref="System.InvalidOperationException">
-        /// Throw when there are still retained references to the memory
-        /// </exception>
-        public void Dispose()
-        {
-            if (IsRetained)
-            {
-                ThrowHelper.ThrowInvalidOperationException_OutstandingReferences();
-            }
-            Dispose(true);
-            GC.SuppressFinalize(this);
-        }
-
-        /// <summary>
-        /// Clean up of any leftover managed and unmanaged resources.
-        /// </summary>
-        protected abstract void Dispose(bool disposing);
-
-        /// <summary>
-        /// Return true if someone is holding a reference to the memory.
-        /// </summary>
-        protected abstract bool IsRetained { get; }
-
-        /// <summary>
-        /// Return true if the underlying memory has been freed.
-        /// </summary>
-        public abstract bool IsDisposed { get; }
-
-        /// <summary>
-        /// Implements IRetainable. Prevent accidental disposal of the memory.
-        /// </summary>
-        public abstract void Retain();
-
-        /// <summary>
-        /// Implements IRetainable. The memory can now be diposed.
-        /// </summary>
-        public abstract bool Release();
-
-    }
-}
diff --git a/src/System.Private.CoreLib/shared/System/IO/FileStreamCompletionSource.Win32.cs b/src/System.Private.CoreLib/shared/System/IO/FileStreamCompletionSource.Win32.cs
index 82274b131..62ace0918 100644
--- a/src/System.Private.CoreLib/shared/System/IO/FileStreamCompletionSource.Win32.cs
+++ b/src/System.Private.CoreLib/shared/System/IO/FileStreamCompletionSource.Win32.cs
@@ -246,7 +246,7 @@ namespace System.IO
             internal MemoryFileStreamCompletionSource(FileStream stream, int numBufferedBytes, ReadOnlyMemory<byte> memory) :
                 base(stream, numBufferedBytes, bytes: null) // this type handles the pinning, so null is passed for bytes
             {
-                _handle = memory.Retain(pin: true);
+                _handle = memory.Pin();
             }
 
             internal override void ReleaseNativeResource()
diff --git a/src/System.Private.CoreLib/shared/System/Memory.cs b/src/System.Private.CoreLib/shared/System/Memory.cs
index bb2b1557a..6eb5af665 100644
--- a/src/System.Private.CoreLib/shared/System/Memory.cs
+++ b/src/System.Private.CoreLib/shared/System/Memory.cs
@@ -26,15 +26,18 @@ namespace System
         // as code uses Unsafe.As to cast between them.
 
         // The highest order bit of _index is used to discern whether _object is an array/string or an owned memory
-        // if (_index >> 31) == 1, object _object is an OwnedMemory<T>
-        // else, object _object is a T[] or a string. It can only be a string if the Memory<T> was created by
+        // if (_index >> 31) == 1, object _object is an MemoryManager<T>
+        // else, object _object is a T[] or a string.
+        //     if (_length >> 31) == 1, _object is a pre-pinned array, so Pin() will not allocate a new GCHandle
+        //     else, Pin() needs to allocate a new GCHandle to pin the object.
+        // It can only be a string if the Memory<T> was created by
         // using unsafe / marshaling code to reinterpret a ReadOnlyMemory<char> wrapped around a string as
         // a Memory<T>.
         private readonly object _object;
         private readonly int _index;
         private readonly int _length;
 
-        private const int RemoveOwnedFlagBitMask = 0x7FFFFFFF;
+        private const int RemoveFlagsBitMask = 0x7FFFFFFF;
 
         /// <summary>
         /// Creates a new memory over the entirety of the target array.
@@ -110,22 +113,72 @@ namespace System
             _length = length;
         }
 
-        // Constructor for internal use only.
+        /// <summary>
+        /// Creates a new memory from a memory manager that provides specific method implementations beginning
+        /// at 'start' index and ending at 'end' index (exclusive).
+        /// </summary>
+        /// <param name="manager">The memory manager.</param>
+        /// <param name="start">The index at which to begin the memory.</param>
+        /// <param name="length">The number of items in the memory.</param>
+        /// <remarks>Returns default when <paramref name="manager"/> is null.</remarks>
+        /// <exception cref="System.ArgumentOutOfRangeException">
+        /// Thrown when the specified <paramref name="start"/> or end index is not in the range (&lt;0 or &gt;=Length).
+        /// </exception>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal Memory(OwnedMemory<T> owner, int index, int length)
+        public Memory(MemoryManager<T> manager, int start, int length)
         {
-            // No validation performed; caller must provide any necessary validation.
-            _object = owner;
-            _index = index | (1 << 31); // Before using _index, check if _index < 0, then 'and' it with RemoveOwnedFlagBitMask
+            if (manager == null)
+            {
+                if (start != 0 || length != 0)
+                    ThrowHelper.ThrowArgumentOutOfRangeException();
+                this = default;
+                return; // returns default
+            }
+            if ((uint)start > (uint)manager.Length || (uint)length > (uint)(manager.Length - start))
+                ThrowHelper.ThrowArgumentOutOfRangeException();
+
+            _object = manager;
+            _index = start | (1 << 31); // Before using _index, check if _index < 0, then 'and' it with RemoveFlagsBitMask
             _length = length;
         }
 
+        /// <summary>
+        /// Creates a new memory over the portion of the pre-pinned target array beginning
+        /// at 'start' index and ending at 'end' index (exclusive).
+        /// </summary>
+        /// <param name="array">The pre-pinned target array.</param>
+        /// <param name="start">The index at which to begin the memory.</param>
+        /// <param name="length">The number of items in the memory.</param>
+        /// <remarks>Returns default when <paramref name="array"/> is null.</remarks>
+        /// <exception cref="System.ArrayTypeMismatchException">Thrown when <paramref name="array"/> is covariant and array's type is not exactly T[].</exception>
+        /// <exception cref="System.ArgumentOutOfRangeException">
+        /// Thrown when the specified <paramref name="start"/> or end index is not in the range (&lt;0 or &gt;=Length).
+        /// </exception>
+        [EditorBrowsable(EditorBrowsableState.Never)]
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private Memory(object obj, int index, int length)
+        public static Memory<T> CreateFromPinnedArray(T[] array, int start, int length)
+        {
+            if (array == null)
+            {
+                if (start != 0 || length != 0)
+                    ThrowHelper.ThrowArgumentOutOfRangeException();
+                return default;
+            }
+            if (default(T) == null && array.GetType() != typeof(T[]))
+                ThrowHelper.ThrowArrayTypeMismatchException();
+            if ((uint)start > (uint)array.Length || (uint)length > (uint)(array.Length - start))
+                ThrowHelper.ThrowArgumentOutOfRangeException();
+
+            // Before using _length, check if _length < 0, then 'and' it with RemoveFlagsBitMask
+            return new Memory<T>((object)array, start, length | (1 << 31));
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private Memory(object obj, int start, int length)
         {
             // No validation performed; caller must provide any necessary validation.
             _object = obj;
-            _index = index;
+            _index = start;
             _length = length;
         }
 
@@ -153,12 +206,12 @@ namespace System
         /// <summary>
         /// The number of items in the memory.
         /// </summary>
-        public int Length => _length;
+        public int Length => _length & RemoveFlagsBitMask;
 
         /// <summary>
         /// Returns true if Length is 0.
         /// </summary>
-        public bool IsEmpty => _length == 0;
+        public bool IsEmpty => (_length & RemoveFlagsBitMask) == 0;
 
         /// <summary>
         /// For <see cref="Memory{Char}"/>, returns a new instance of string that represents the characters pointed to by the memory.
@@ -168,9 +221,9 @@ namespace System
         {
             if (typeof(T) == typeof(char))
             {
-                return (_object is string str) ? str.Substring(_index, _length) : Span.ToString();
+                return (_object is string str) ? str.Substring(_index, _length & RemoveFlagsBitMask) : Span.ToString();
             }
-            return string.Format("System.Memory<{0}>[{1}]", typeof(T).Name, _length);
+            return string.Format("System.Memory<{0}>[{1}]", typeof(T).Name, _length & RemoveFlagsBitMask);
         }
 
         /// <summary>
@@ -183,12 +236,13 @@ namespace System
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public Memory<T> Slice(int start)
         {
-            if ((uint)start > (uint)_length)
+            int actualLength = _length & RemoveFlagsBitMask;
+            if ((uint)start > (uint)actualLength)
             {
                 ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
             }
 
-            return new Memory<T>(_object, _index + start, _length - start);
+            return new Memory<T>(_object, _index + start, actualLength - start);
         }
 
         /// <summary>
@@ -202,7 +256,8 @@ namespace System
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public Memory<T> Slice(int start, int length)
         {
-            if ((uint)start > (uint)_length || (uint)length > (uint)(_length - start))
+            int actualLength = _length & RemoveFlagsBitMask;
+            if ((uint)start > (uint)actualLength || (uint)length > (uint)(actualLength - start))
             {
                 ThrowHelper.ThrowArgumentOutOfRangeException();
             }
@@ -220,10 +275,12 @@ namespace System
             {
                 if (_index < 0)
                 {
-                    return ((OwnedMemory<T>)_object).Span.Slice(_index & RemoveOwnedFlagBitMask, _length);
+                    Debug.Assert(_length >= 0);
+                    return ((MemoryManager<T>)_object).GetSpan().Slice(_index & RemoveFlagsBitMask, _length);
                 }
                 else if (typeof(T) == typeof(char) && _object is string s)
                 {
+                    Debug.Assert(_length >= 0);
                     // This is dangerous, returning a writable span for a string that should be immutable.
                     // However, we need to handle the case where a ReadOnlyMemory<char> was created from a string
                     // and then cast to a Memory<T>. Such a cast can only be done with unsafe or marshaling code,
@@ -237,7 +294,7 @@ namespace System
                 }
                 else if (_object != null)
                 {
-                    return new Span<T>((T[])_object, _index, _length);
+                    return new Span<T>((T[])_object, _index, _length & RemoveFlagsBitMask);
                 }
                 else
                 {
@@ -278,7 +335,7 @@ namespace System
         {
             if (_index < 0)
             {
-                return ((OwnedMemory<T>)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
+                return ((MemoryManager<T>)_object).Pin((_index & RemoveFlagsBitMask));
             }
             else if (typeof(T) == typeof(char) && _object is string s)
             {
@@ -293,72 +350,21 @@ namespace System
 #else
                 void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
 #endif // FEATURE_PORTABLE_SPAN
-                return new MemoryHandle(null, pointer, handle);
+                return new MemoryHandle(pointer, handle);
             }
             else if (_object is T[] array)
             {
-                var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
+                GCHandle handle = _length < 0 ? default : GCHandle.Alloc(array, GCHandleType.Pinned);
 #if FEATURE_PORTABLE_SPAN
                 void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
 #else
                 void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
 #endif // FEATURE_PORTABLE_SPAN
-                return new MemoryHandle(null, pointer, handle);
+                return new MemoryHandle(pointer, handle);
             }
             return default;
         }
 
-        /// <summary>[Obsolete, use Pin()] Creates a handle for the memory.</summary>
-        /// <param name="pin">
-        /// If pin is true, the GC will not move the array until the returned <see cref="MemoryHandle"/>
-        /// is disposed, enabling taking and using the memory's address.
-        /// </param>
-        public unsafe MemoryHandle Retain(bool pin = false)
-        {
-            MemoryHandle memoryHandle = default;
-            if (pin)
-            {
-                if (_index < 0)
-                {
-                    memoryHandle = ((OwnedMemory<T>)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
-                }
-                else if (typeof(T) == typeof(char) && _object is string s)
-                {
-                    // This case can only happen if a ReadOnlyMemory<char> was created around a string
-                    // and then that was cast to a Memory<char> using unsafe / marshaling code.  This needs
-                    // to work, however, so that code that uses a single Memory<char> field to store either
-                    // a readable ReadOnlyMemory<char> or a writable Memory<char> can still be pinned and
-                    // used for interop purposes.
-                    GCHandle handle = GCHandle.Alloc(s, GCHandleType.Pinned);
-#if FEATURE_PORTABLE_SPAN
-                    void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
-#else
-                    void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
-#endif // FEATURE_PORTABLE_SPAN
-                    memoryHandle = new MemoryHandle(null, pointer, handle);
-                }
-                else if (_object is T[] array)
-                {
-                    var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
-#if FEATURE_PORTABLE_SPAN
-                    void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
-#else
-                    void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
-#endif // FEATURE_PORTABLE_SPAN
-                    memoryHandle = new MemoryHandle(null, pointer, handle);
-                }
-            }
-            else
-            {
-                if (_index < 0)
-                {
-                    ((OwnedMemory<T>)_object).Retain();
-                    memoryHandle = new MemoryHandle((OwnedMemory<T>)_object);
-                }
-            }
-            return memoryHandle;
-        }
-
         /// <summary>
         /// Copies the contents from the memory into a new array.  This heap
         /// allocates, so should generally be avoided, however it is sometimes
diff --git a/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs b/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs
index dca7db3df..ca0e7d888 100644
--- a/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs
+++ b/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs
@@ -26,13 +26,15 @@ namespace System
         // as code uses Unsafe.As to cast between them.
 
         // The highest order bit of _index is used to discern whether _object is an array/string or an owned memory
-        // if (_index >> 31) == 1, _object is an OwnedMemory<T>
-        // else, _object is a T[] or string
+        // if (_index >> 31) == 1, _object is an MemoryManager<T>
+        // else, _object is a T[] or string.
+        //     if (_length >> 31) == 1, _object is a pre-pinned array, so Pin() will not allocate a new GCHandle
+        //     else, Pin() needs to allocate a new GCHandle to pin the object.
         private readonly object _object;
         private readonly int _index;
         private readonly int _length;
 
-        internal const int RemoveOwnedFlagBitMask = 0x7FFFFFFF;
+        internal const int RemoveFlagsBitMask = 0x7FFFFFFF;
 
         /// <summary>
         /// Creates a new memory over the entirety of the target array.
@@ -115,12 +117,12 @@ namespace System
         /// <summary>
         /// The number of items in the memory.
         /// </summary>
-        public int Length => _length;
+        public int Length => _length & RemoveFlagsBitMask;
 
         /// <summary>
         /// Returns true if Length is 0.
         /// </summary>
-        public bool IsEmpty => _length == 0;
+        public bool IsEmpty => (_length & RemoveFlagsBitMask) == 0;
 
         /// <summary>
         /// For <see cref="ReadOnlyMemory{Char}"/>, returns a new instance of string that represents the characters pointed to by the memory.
@@ -130,9 +132,9 @@ namespace System
         {
             if (typeof(T) == typeof(char))
             {
-                return (_object is string str) ? str.Substring(_index, _length) : Span.ToString();
+                return (_object is string str) ? str.Substring(_index, _length & RemoveFlagsBitMask) : Span.ToString();
             }
-            return string.Format("System.ReadOnlyMemory<{0}>[{1}]", typeof(T).Name, _length);
+            return string.Format("System.ReadOnlyMemory<{0}>[{1}]", typeof(T).Name, _length & RemoveFlagsBitMask);
         }
 
         /// <summary>
@@ -145,12 +147,13 @@ namespace System
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public ReadOnlyMemory<T> Slice(int start)
         {
-            if ((uint)start > (uint)_length)
+            int actualLength = _length & RemoveFlagsBitMask;
+            if ((uint)start > (uint)actualLength)
             {
                 ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
             }
 
-            return new ReadOnlyMemory<T>(_object, _index + start, _length - start);
+            return new ReadOnlyMemory<T>(_object, _index + start, actualLength - start);
         }
 
         /// <summary>
@@ -164,7 +167,8 @@ namespace System
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public ReadOnlyMemory<T> Slice(int start, int length)
         {
-            if ((uint)start > (uint)_length || (uint)length > (uint)(_length - start))
+            int actualLength = _length & RemoveFlagsBitMask;
+            if ((uint)start > (uint)actualLength || (uint)length > (uint)(actualLength - start))
             {
                 ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
             }
@@ -182,10 +186,12 @@ namespace System
             {
                 if (_index < 0)
                 {
-                    return ((OwnedMemory<T>)_object).Span.Slice(_index & RemoveOwnedFlagBitMask, _length);
+                    Debug.Assert(_length >= 0);
+                    return ((MemoryManager<T>)_object).GetSpan().Slice(_index & RemoveFlagsBitMask, _length);
                 }
                 else if (typeof(T) == typeof(char) && _object is string s)
                 {
+                    Debug.Assert(_length >= 0);
 #if FEATURE_PORTABLE_SPAN
                     return new ReadOnlySpan<T>(Unsafe.As<Pinnable<T>>(s), MemoryExtensions.StringAdjustment, s.Length).Slice(_index, _length);
 #else
@@ -194,7 +200,7 @@ namespace System
                 }
                 else if (_object != null)
                 {
-                    return new ReadOnlySpan<T>((T[])_object, _index, _length);
+                    return new ReadOnlySpan<T>((T[])_object, _index, _length & RemoveFlagsBitMask);
                 }
                 else
                 {
@@ -235,7 +241,7 @@ namespace System
         {
             if (_index < 0)
             {
-                return ((OwnedMemory<T>)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
+                return ((MemoryManager<T>)_object).Pin((_index & RemoveFlagsBitMask));
             }
             else if (typeof(T) == typeof(char) && _object is string s)
             {
@@ -245,67 +251,21 @@ namespace System
 #else
                 void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
 #endif // FEATURE_PORTABLE_SPAN
-                return new MemoryHandle(null, pointer, handle);
+                return new MemoryHandle(pointer, handle);
             }
             else if (_object is T[] array)
             {
-                var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
+                GCHandle handle = _length < 0 ? default : GCHandle.Alloc(array, GCHandleType.Pinned);
 #if FEATURE_PORTABLE_SPAN
                 void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
 #else
                 void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
 #endif // FEATURE_PORTABLE_SPAN
-                return new MemoryHandle(null, pointer, handle);
+                return new MemoryHandle(pointer, handle);
             }
             return default;
         }
 
-        /// <summary>[Obsolete, use Pin()] Creates a handle for the memory.</summary>
-        /// <param name="pin">
-        /// If pin is true, the GC will not move the array until the returned <see cref="MemoryHandle"/>
-        /// is disposed, enabling taking and using the memory's address.
-        /// </param>
-        public unsafe MemoryHandle Retain(bool pin = false)
-        {
-            MemoryHandle memoryHandle = default;
-            if (pin)
-            {
-                if (_index < 0)
-                {
-                    memoryHandle = ((OwnedMemory<T>)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
-                }
-                else if (typeof(T) == typeof(char) && _object is string s)
-                {
-                    GCHandle handle = GCHandle.Alloc(s, GCHandleType.Pinned);
-#if FEATURE_PORTABLE_SPAN
-                    void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
-#else
-                    void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
-#endif // FEATURE_PORTABLE_SPAN
-                    memoryHandle = new MemoryHandle(null, pointer, handle);
-                }
-                else if (_object is T[] array)
-                {
-                    var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
-#if FEATURE_PORTABLE_SPAN
-                    void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
-#else
-                    void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
-#endif // FEATURE_PORTABLE_SPAN
-                    memoryHandle = new MemoryHandle(null, pointer, handle);
-                }
-            }
-            else
-            {
-                if (_index < 0)
-                {
-                    ((OwnedMemory<T>)_object).Retain();
-                    memoryHandle = new MemoryHandle((OwnedMemory<T>)_object);
-                }
-            }
-            return memoryHandle;
-        }
-
         /// <summary>
         /// Copies the contents from the memory into a new array.  This heap
         /// allocates, so should generally be avoided, however it is sometimes
diff --git a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/MemoryMarshal.cs b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/MemoryMarshal.cs
index b4b17b037..073dd76cd 100644
--- a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/MemoryMarshal.cs
+++ b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/MemoryMarshal.cs
@@ -5,6 +5,7 @@
 using System.Buffers;
 using System.Runtime.CompilerServices;
 using System.Collections.Generic;
+using System.Diagnostics;
 
 #if !netstandard
 using Internal.Runtime.CompilerServices;
@@ -27,19 +28,20 @@ namespace System.Runtime.InteropServices
             object obj = memory.GetObjectStartLength(out int index, out int length);
             if (index < 0)
             {
-                if (((OwnedMemory<T>)obj).TryGetArray(out ArraySegment<T> arraySegment))
+                Debug.Assert(length >= 0);
+                if (((MemoryManager<T>)obj).TryGetArray(out ArraySegment<T> arraySegment))
                 {
-                    segment = new ArraySegment<T>(arraySegment.Array, arraySegment.Offset + (index & ReadOnlyMemory<T>.RemoveOwnedFlagBitMask), length);
+                    segment = new ArraySegment<T>(arraySegment.Array, arraySegment.Offset + (index & ReadOnlyMemory<T>.RemoveFlagsBitMask), length);
                     return true;
                 }
             }
             else if (obj is T[] arr)
             {
-                segment = new ArraySegment<T>(arr, index, length);
+                segment = new ArraySegment<T>(arr, index, length & ReadOnlyMemory<T>.RemoveFlagsBitMask);
                 return true;
             }
 
-            if (length == 0)
+            if ((length & ReadOnlyMemory<T>.RemoveFlagsBitMask) == 0)
             {
 #if FEATURE_PORTABLE_SPAN
                 segment = new ArraySegment<T>(SpanHelpers.PerTypeValues<T>.EmptyArray);
@@ -54,40 +56,49 @@ namespace System.Runtime.InteropServices
         }
 
         /// <summary>
-        /// Gets an <see cref="OwnedMemory{T}"/> from the underlying read-only memory.
-        /// If unable to get the <typeparamref name="TOwner"/> type, returns false.
+        /// Gets an <see cref="MemoryManager{T}"/> from the underlying read-only memory.
+        /// If unable to get the <typeparamref name="TManager"/> type, returns false.
         /// </summary>
         /// <typeparam name="T">The element type of the <paramref name="memory" />.</typeparam>
-        /// <typeparam name="TOwner">The type of <see cref="OwnedMemory{T}"/> to try and retrive.</typeparam>
-        /// <param name="memory">The memory to get the owner for.</param>
-        /// <param name="owner">The returned owner of the <see cref="ReadOnlyMemory{T}"/>.</param>
+        /// <typeparam name="TManager">The type of <see cref="MemoryManager{T}"/> to try and retrive.</typeparam>
+        /// <param name="memory">The memory to get the manager for.</param>
+        /// <param name="manager">The returned manager of the <see cref="ReadOnlyMemory{T}"/>.</param>
         /// <returns>A <see cref="bool"/> indicating if it was successful.</returns>
-        public static bool TryGetOwnedMemory<T, TOwner>(ReadOnlyMemory<T> memory, out TOwner owner)
-            where TOwner : OwnedMemory<T>
+        public static bool TryGetMemoryManager<T, TManager>(ReadOnlyMemory<T> memory, out TManager manager)
+            where TManager : MemoryManager<T>
         {
-            TOwner localOwner; // Use register for null comparison rather than byref
-            owner = localOwner = memory.GetObjectStartLength(out int index, out int length) as TOwner;
-            return !ReferenceEquals(owner, null);
+            TManager localManager; // Use register for null comparison rather than byref
+            manager = localManager = memory.GetObjectStartLength(out _, out _) as TManager;
+            return !ReferenceEquals(manager, null);
         }
 
         /// <summary>
-        /// Gets an <see cref="OwnedMemory{T}"/> and <paramref name="start" />, <paramref name="length" /> from the underlying read-only memory.
-        /// If unable to get the <typeparamref name="TOwner"/> type, returns false.
+        /// Gets an <see cref="MemoryManager{T}"/> and <paramref name="start" />, <paramref name="length" /> from the underlying read-only memory.
+        /// If unable to get the <typeparamref name="TManager"/> type, returns false.
         /// </summary>
         /// <typeparam name="T">The element type of the <paramref name="memory" />.</typeparam>
-        /// <typeparam name="TOwner">The type of <see cref="OwnedMemory{T}"/> to try and retrive.</typeparam>
-        /// <param name="memory">The memory to get the owner for.</param>
-        /// <param name="owner">The returned owner of the <see cref="ReadOnlyMemory{T}"/>.</param>
-        /// <param name="start">The offset from the start of the <paramref name="owner" /> that the <paramref name="memory" /> represents.</param>
-        /// <param name="length">The length of the <paramref name="owner" /> that the <paramref name="memory" /> represents.</param>
+        /// <typeparam name="TManager">The type of <see cref="MemoryManager{T}"/> to try and retrive.</typeparam>
+        /// <param name="memory">The memory to get the manager for.</param>
+        /// <param name="manager">The returned manager of the <see cref="ReadOnlyMemory{T}"/>.</param>
+        /// <param name="start">The offset from the start of the <paramref name="manager" /> that the <paramref name="memory" /> represents.</param>
+        /// <param name="length">The length of the <paramref name="manager" /> that the <paramref name="memory" /> represents.</param>
         /// <returns>A <see cref="bool"/> indicating if it was successful.</returns>
-        public static bool TryGetOwnedMemory<T, TOwner>(ReadOnlyMemory<T> memory, out TOwner owner, out int start, out int length)
-           where TOwner : OwnedMemory<T>
+        public static bool TryGetMemoryManager<T, TManager>(ReadOnlyMemory<T> memory, out TManager manager, out int start, out int length)
+           where TManager : MemoryManager<T>
         {
-            TOwner localOwner; // Use register for null comparison rather than byref
-            owner = localOwner = memory.GetObjectStartLength(out start, out length) as TOwner;
-            start &= ReadOnlyMemory<T>.RemoveOwnedFlagBitMask;
-            return !ReferenceEquals(owner, null);
+            TManager localManager; // Use register for null comparison rather than byref
+            manager = localManager = memory.GetObjectStartLength(out start, out length) as TManager;
+            start &= ReadOnlyMemory<T>.RemoveFlagsBitMask;
+
+            Debug.Assert(length >= 0);
+
+            if (ReferenceEquals(manager, null))
+            {
+                start = default;
+                length = default;
+                return false;
+            }
+            return true;
         }
 
         /// <summary>
@@ -113,6 +124,8 @@ namespace System.Runtime.InteropServices
         {
             if (memory.GetObjectStartLength(out int offset, out int count) is string s)
             {
+                Debug.Assert(offset >= 0);
+                Debug.Assert(count >= 0);
                 text = s;
                 start = offset;
                 length = count;