feedback from pull request

Author: bollhals

projects/RabbitMQ.Client/client/impl/Command.cs

@@ -77,6 +77,8 @@ internal void Transmit(ushort channelNumber, Connection connection)
         {
             int maxBodyPayloadBytes = (int)(connection.FrameMax == 0 ? int.MaxValue : connection.FrameMax - EmptyFrameSize);
             var size = GetMaxSize(maxBodyPayloadBytes);
+
+            // Will be returned by SocketFrameWriter.WriteLoop
             var memory = new Memory<byte>(ArrayPool<byte>.Shared.Rent(size), 0, size);
             var span = memory.Span;
 

projects/RabbitMQ.Client/client/impl/CommandAssembler.cs

@@ -100,6 +100,8 @@ public Command HandleFrame(in InboundFrame f)
                     }
 
                     m_remainingBodyBytes = (int)totalBodyBytes;
+
+                    // Is returned by Command.Dispose in Session.HandleFrame
                     byte[] bodyBytes = ArrayPool<byte>.Shared.Rent(m_remainingBodyBytes);
                     m_body = new Memory<byte>(bodyBytes, 0, m_remainingBodyBytes);
                     UpdateContentBodyState();

projects/RabbitMQ.Client/client/impl/Connection.cs

@@ -903,9 +903,7 @@ public void HeartbeatWriteTimerCallback(object state)
             {
                 if (!_closed)
                 {
-                    var memory = new Memory<byte>(ArrayPool<byte>.Shared.Rent(Client.Impl.Framing.Heartbeat.FrameSize), 0, Client.Impl.Framing.Heartbeat.FrameSize);
-                    Client.Impl.Framing.Heartbeat.Payload.CopyTo(memory.Span);
-                    Write(memory);
+                    Write(Client.Impl.Framing.Heartbeat.GetHeartbeatFrame());
                     _heartbeatWriteTimer?.Change((int)_heartbeatTimeSpan.TotalMilliseconds, Timeout.Infinite);
                 }
             }

projects/RabbitMQ.Client/client/impl/Frame.cs

@@ -145,14 +145,24 @@ internal static class Heartbeat
             /* Empty frame */
             public const int FrameSize = BaseFrameSize;
 
-            public static Span<byte> Payload => new byte[]
+            /// <summary>
+            /// Compiler trick to directly refer to static data in the assembly, see here: https://github.com/dotnet/roslyn/pull/24621
+            /// </summary>
+            private static ReadOnlySpan<byte> Payload => new byte[]
             {
                 Constants.FrameHeartbeat,
                 0, 0, // channel
                 0, 0, 0, 0, // payload length
                 Constants.FrameEnd
             };
 
+            public static Memory<byte> GetHeartbeatFrame()
+            {
+                // Is returned by SocketFrameHandler.WriteLoop
+                var buffer = ArrayPool<byte>.Shared.Rent(FrameSize);
+                Payload.CopyTo(buffer);
+                return new Memory<byte>(buffer, 0, FrameSize);
+            }
         }
     }
 
@@ -254,6 +264,8 @@ internal static InboundFrame ReadFrom(Stream reader)
             reader.Read(headerBytes);
             int channel = NetworkOrderDeserializer.ReadUInt16(headerBytes);
             int payloadSize = NetworkOrderDeserializer.ReadInt32(headerBytes.Slice(2)); // FIXME - throw exn on unreasonable value
+
+            // Is returned by InboundFrame.Dispose in Connection.MainLoopIteration
             byte[] payloadBytes = ArrayPool<byte>.Shared.Rent(payloadSize);
             Memory<byte> payload = new Memory<byte>(payloadBytes, 0, payloadSize);
             int bytesRead = 0;

projects/RabbitMQ.Client/client/impl/SocketFrameHandler.cs

@@ -200,7 +200,7 @@ public void Close()
                     try
                     {
                         _channelWriter.Complete();
-                        _writerTask.Wait();
+                        _writerTask.GetAwaiter().GetResult();
                     }
                     catch(Exception)
                     {

projects/Unit/TestFrameFormatting.cs

@@ -38,6 +38,9 @@
 //  Copyright (c) 2007-2020 VMware, Inc.  All rights reserved.
 //---------------------------------------------------------------------------
 
+using System;
+using System.Buffers;
+using System.Runtime.InteropServices;
 using NUnit.Framework;
 using RabbitMQ.Client.Framing.Impl;
 
@@ -49,17 +52,28 @@ class TestFrameFormatting : WireFormattingFixture
         [Test]
         public void HeartbeatFrame()
         {
-            var frameBytes = Impl.Framing.Heartbeat.Payload.ToArray();
+            var memory = Impl.Framing.Heartbeat.GetHeartbeatFrame();
+            var frameSpan = memory.Span;
 
-            Assert.AreEqual(8, frameBytes.Length);
-            Assert.AreEqual(Constants.FrameHeartbeat, frameBytes[0]);
-            Assert.AreEqual(0, frameBytes[1]); // channel
-            Assert.AreEqual(0, frameBytes[2]); // channel
-            Assert.AreEqual(0, frameBytes[3]); // payload size
-            Assert.AreEqual(0, frameBytes[4]); // payload size
-            Assert.AreEqual(0, frameBytes[5]); // payload size
-            Assert.AreEqual(0, frameBytes[6]); // payload size
-            Assert.AreEqual(Constants.FrameEnd, frameBytes[7]);
+            try
+            {
+                Assert.AreEqual(8, frameSpan.Length);
+                Assert.AreEqual(Constants.FrameHeartbeat, frameSpan[0]);
+                Assert.AreEqual(0, frameSpan[1]); // channel
+                Assert.AreEqual(0, frameSpan[2]); // channel
+                Assert.AreEqual(0, frameSpan[3]); // payload size
+                Assert.AreEqual(0, frameSpan[4]); // payload size
+                Assert.AreEqual(0, frameSpan[5]); // payload size
+                Assert.AreEqual(0, frameSpan[6]); // payload size
+                Assert.AreEqual(Constants.FrameEnd, frameSpan[7]);
+            }
+            finally
+            {
+                if (MemoryMarshal.TryGetArray(memory, out ArraySegment<byte> segment))
+                {
+                    ArrayPool<byte>.Shared.Return(segment.Array);
+                }
+            }
         }
 
         [Test]