ResourceLeakDetector

src/DotNetty.Buffers/ByteBufferUtil.cs

@@ -360,7 +360,7 @@ namespace DotNetty.Buffers
                 try
                 {
                     buffer.WriteBytes(src, readerIndex, len);
-                    ArraySegment<byte> ioBuf = src.GetIoBuffer();
+                    ArraySegment<byte> ioBuf = buffer.GetIoBuffer();
                     return encoding.GetString(ioBuf.Array, ioBuf.Offset, ioBuf.Count);
                 }
                 finally

src/DotNetty.Buffers/CompositeByteBuffer.cs

@@ -1152,7 +1152,7 @@ namespace DotNetty.Buffers
         public override IByteBuffer Copy(int index, int length)
         {
             this.CheckIndex(index, length);
-            IByteBuffer dst = Unpooled.Buffer(length);
+            IByteBuffer dst = this.AllocateBuffer(length);
             if (length != 0)
             {
                 this.CopyTo(index, length, this.ToComponentIndex(index), dst);

src/DotNetty.Buffers/EmptyByteBuffer.cs

@@ -651,10 +651,6 @@ namespace DotNetty.Buffers
 
         public int ArrayOffset => 0;
 
-        public bool HasMemoryAddress => false;
-
-        public unsafe byte* MemoryAddress => throw new NotSupportedException();
-
         public string ToString(Encoding encoding) => string.Empty;
 
         public string ToString(int index, int length, Encoding encoding)

src/DotNetty.Common/ResourceLeakDetector.cs

@@ -14,17 +14,16 @@ namespace DotNetty.Common
     using DotNetty.Common.Internal;
     using DotNetty.Common.Internal.Logging;
     using DotNetty.Common.Utilities;
-    using Nito;
 
-    public sealed class ResourceLeakDetector
+    public class ResourceLeakDetector
     {
         const string PropLevel = "io.netty.leakDetection.level";
         const DetectionLevel DefaultLevel = DetectionLevel.Simple;
 
-        const string PropMaxRecords = "io.netty.leakDetection.maxRecords";
-        const int DefaultMaxRecords = 4;
+        const string PropTargetRecords = "io.netty.leakDetection.targetRecords";
+        const int DefaultTargetRecords = 4;
 
-        static readonly int MaxRecords;
+        static readonly int TargetRecords;
 
         /// <summary>
         ///    Represents the level of resource leak detection.
@@ -66,13 +65,13 @@ namespace DotNetty.Common
                 level = DefaultLevel;
             }
 
-            MaxRecords = SystemPropertyUtil.GetInt(PropMaxRecords, DefaultMaxRecords);
-
+            TargetRecords = SystemPropertyUtil.GetInt(PropTargetRecords, DefaultTargetRecords);
             Level = level;
+
             if (Logger.DebugEnabled)
             {
-                Logger.Debug("{}: {}", PropLevel, level.ToString().ToLowerInvariant());
-                Logger.Debug("{}: {}", PropMaxRecords, MaxRecords);
+                Logger.Debug("-D{}: {}", PropLevel, level.ToString().ToLower());
+                Logger.Debug("-D{}: {}", PropTargetRecords, TargetRecords);
             }
         }
 
@@ -122,6 +121,7 @@ namespace DotNetty.Common
             {
                 return null;
             }
+
             if (level < DetectionLevel.Paranoid)
             {
                 if ((PlatformDependent.GetThreadLocalRandom().Next(this.samplingInterval)) == 0)
@@ -139,34 +139,46 @@ namespace DotNetty.Common
             }
         }
 
-        internal void Report(IResourceLeakTracker resourceLeak)
+        void ReportLeak(DefaultResourceLeak resourceLeak)
         {
             string records = resourceLeak.ToString();
             if (this.reportedLeaks.TryAdd(records, true))
             {
                 if (records.Length == 0)
                 {
-                    Logger.Error("LEAK: {}.Release() was not called before it's garbage-collected. " +
-                        "Enable advanced leak reporting to find out where the leak occurred. " +
-                        "To enable advanced leak reporting, " +
-                        "set environment variable {} to {} or set {}.Level in code. " +
-                        "See http://netty.io/wiki/reference-counted-objects.html for more information.", this.resourceType, PropLevel, DetectionLevel.Advanced.ToString().ToLowerInvariant(), StringUtil.SimpleClassName(this));
+                    this.ReportUntracedLeak(this.resourceType);
                 }
                 else
                 {
-                    Logger.Error(
-                        "LEAK: {}.release() was not called before it's garbage-collected. " +
-                            "See http://netty.io/wiki/reference-counted-objects.html for more information.{}", this.resourceType, records);
+                    this.ReportTracedLeak(this.resourceType, records);
                 }
             }
         }
 
+        protected void ReportTracedLeak(string type, string records)
+        {
+            Logger.Error(
+                "LEAK: {}.Release() was not called before it's garbage-collected. " +
+                "See http://netty.io/wiki/reference-counted-objects.html for more information.{}",
+                type, records);
+        }
+
+        protected void ReportUntracedLeak(string type)
+        {
+            Logger.Error("LEAK: {}.release() was not called before it's garbage-collected. " +
+                "Enable advanced leak reporting to find out where the leak occurred. " +
+                "To enable advanced leak reporting, " +
+                "specify the JVM option '-D{}={}' or call {}.setLevel() " +
+                "See http://netty.io/wiki/reference-counted-objects.html for more information.",
+                type, PropLevel, DetectionLevel.Advanced.ToString().ToLower(), StringUtil.SimpleClassName(this));
+        }
+
         sealed class DefaultResourceLeak : IResourceLeakTracker
         {
             readonly ResourceLeakDetector owner;
-            readonly string creationRecord;
-            readonly Deque<string> lastRecords = new Deque<string>();
-            int removedRecords;
+
+            RecordEntry head;
+            long droppedRecords;
 
             public DefaultResourceLeak(ResourceLeakDetector owner, object referent)
             {
@@ -181,137 +193,189 @@ namespace DotNetty.Common
                 {
                     owner.gcNotificationMap.Add(referent, new GCNotice(this, referent));
                 }
-
-                DetectionLevel level = Level;
-                if (level >= DetectionLevel.Advanced)
-                {
-                    this.creationRecord = NewRecord(null);
-                }
-                else
-                {
-                    this.creationRecord = null;
-                }
+                this.head = RecordEntry.Bottom;
             }
 
-            public void Record() => this.RecordInternal(null);
+            public void Record() => this.Record0(null);
 
-            public void Record(object hint) => this.RecordInternal(hint);
+            public void Record(object hint) => this.Record0(hint);
 
-            void RecordInternal(object hint)
+            void Record0(object hint)
             {
-                if (this.creationRecord != null && MaxRecords > 0)
+                // Check TARGET_RECORDS > 0 here to avoid similar check before remove from and add to lastRecords
+                if (TargetRecords > 0)
                 {
-                    string value = NewRecord(hint);
+                    string stackTrace = Environment.StackTrace;
 
-                    lock (this.lastRecords)
+                    RecordEntry oldHead;
+                    RecordEntry prevHead;
+                    RecordEntry newHead;
+                    bool dropped;
+                    do
                     {
-                        int size = this.lastRecords.Count;
-                        if (size == 0 || this.lastRecords[size - 1].Equals(value))
+                        if ((prevHead = oldHead = this.head) == null)
                         {
-                            if (size > MaxRecords)
-                            {
-                                this.lastRecords.RemoveFromFront();
-                                ++this.removedRecords;
-                            }
-                            this.lastRecords.AddToBack(value);
+                            // already closed.
+                            return;
                         }
+                        int numElements = oldHead.Pos + 1;
+                        if (numElements >= TargetRecords)
+                        {
+                            int backOffFactor = Math.Min(numElements - TargetRecords, 30);
+                            dropped = PlatformDependent.GetThreadLocalRandom().Next(1 << backOffFactor) != 0;
+                            if (dropped)
+                            {
+                                prevHead = oldHead.Next;
+                            }
+                        }
+                        else
+                        {
+                            dropped = false;
+                        }
+                        newHead = hint != null ? new RecordEntry(prevHead, stackTrace, hint) : new RecordEntry(prevHead, stackTrace);
+                    }
+                    while (Interlocked.CompareExchange(ref this.head, newHead, oldHead) != oldHead);
+                    if (dropped)
+                    {
+                        Interlocked.Increment(ref this.droppedRecords);
                     }
                 }
             }
 
             public bool Close(object trackedObject)
             {
-                return this.owner.gcNotificationMap.Remove(trackedObject);
+                if (this.owner.gcNotificationMap.Remove(trackedObject))
+                {
+                    Interlocked.Exchange(ref this.head, null);
+                    return true;
+                }
+                return false;
             }
 
+            // This is called from GCNotice finalizer 
             internal void CloseFinal(object trackedObject)
             {
-                if (this.Close(trackedObject))
+                if (this.owner.gcNotificationMap.Remove(trackedObject) 
+                    && Volatile.Read(ref this.head) != null)
                 {
-                    this.owner.Report(this);
+                    this.owner.ReportLeak(this);
                 }
             }
 
             public override string ToString()
             {
-                if (this.creationRecord == null)
+                RecordEntry oldHead = Interlocked.Exchange(ref this.head, null);
+                if (oldHead == null)
                 {
-                    return string.Empty;
+                    // Already closed
+                    return  string.Empty;
                 }
 
-                string[] array;
-                int removed;
-                lock (this.lastRecords)
+                long dropped = Interlocked.Read(ref this.droppedRecords);
+                int duped = 0;
+
+                int present = oldHead.Pos + 1;
+                // Guess about 2 kilobytes per stack trace
+                var buf = new StringBuilder(present * 2048);
+                buf.Append(StringUtil.Newline);
+                buf.Append("Recent access records: ").Append(StringUtil.Newline);
+
+                int i = 1;
+                var seen = new HashSet<string>();
+                for (; oldHead != RecordEntry.Bottom; oldHead = oldHead.Next)
                 {
-                    array = new string[this.lastRecords.Count];
-                    ((ICollection<string>)this.lastRecords).CopyTo(array, 0);
-                    removed = this.removedRecords;
+                    string s = oldHead.ToString();
+                    if (seen.Add(s))
+                    {
+                        if (oldHead.Next == RecordEntry.Bottom)
+                        {
+                            buf.Append("Created at:").Append(StringUtil.Newline).Append(s);
+                        }
+                        else
+                        {
+                            buf.Append('#').Append(i++).Append(':').Append(StringUtil.Newline).Append(s);
+                        }
+                    }
+                    else
+                    {
+                        duped++;
+                    }
                 }
 
-                StringBuilder buf = new StringBuilder(16384).Append(StringUtil.Newline);
-                if (removed > 0)
+                if (duped > 0)
                 {
-                    buf.Append("WARNING: ")
-                        .Append(removed)
-                        .Append(" leak records were discarded because the leak record count is limited to ")
-                        .Append(MaxRecords)
+                    buf.Append(": ")
+                        .Append(dropped)
+                        .Append(" leak records were discarded because they were duplicates")
+                        .Append(StringUtil.Newline);
+                }
+
+                if (dropped > 0)
+                {
+                    buf.Append(": ")
+                        .Append(dropped)
+                        .Append(" leak records were discarded because the leak record count is targeted to ")
+                        .Append(TargetRecords)
                         .Append(". Use system property ")
-                        .Append(PropMaxRecords)
+                        .Append(PropTargetRecords)
                         .Append(" to increase the limit.")
                         .Append(StringUtil.Newline);
                 }
 
-                buf.Append("Recent access records: ")
-                    .Append(array.Length)
-                    .Append(StringUtil.Newline);
-
-                if (array.Length > 0)
-                {
-                    for (int i = array.Length - 1; i >= 0; i--)
-                    {
-                        buf.Append('#')
-                            .Append(i + 1)
-                            .Append(':')
-                            .Append(StringUtil.Newline)
-                            .Append(array[i]);
-                    }
-                    buf.Append(StringUtil.Newline);
-                }
-
-                buf.Append("Created at:")
-                    .Append(StringUtil.Newline)
-                    .Append(this.creationRecord);
-
+                buf.Length = buf.Length - StringUtil.Newline.Length;
                 return buf.ToString();
             }
         }
 
-        static string NewRecord(object hint)
+        // Record
+        sealed class RecordEntry
         {
-            Contract.Ensures(Contract.Result<string>() != null);
+            internal static readonly RecordEntry Bottom = new RecordEntry();
 
-            var buf = new StringBuilder(4096);
+            readonly string hintString;
+            internal readonly RecordEntry Next;
+            internal readonly int Pos;
+            readonly string stackTrace;
 
-            // Append the hint first if available.
-            if (hint != null)
+            internal RecordEntry(RecordEntry next, string stackTrace, object hint)
             {
-                buf.Append("\tHint: ");
-                // Prefer a hint string to a simple string form.
-                var leakHint = hint as IResourceLeakHint;
-                if (leakHint != null)
-                {
-                    buf.Append(leakHint.ToHintString());
-                }
-                else
-                {
-                    buf.Append(hint);
-                }
-                buf.Append(StringUtil.Newline);
+                // This needs to be generated even if toString() is never called as it may change later on.
+                this.hintString = hint is IResourceLeakHint leakHint ? leakHint.ToHintString() : null;
+                this.Next = next;
+                this.Pos = next.Pos + 1;
+                this.stackTrace = stackTrace;
             }
 
-            // Append the stack trace.
-            buf.Append(Environment.StackTrace);
-            return buf.ToString();
+            internal RecordEntry(RecordEntry next, string stackTrace)
+            {
+                this.hintString = null;
+                this.Next = next;
+                this.Pos = next.Pos + 1;
+                this.stackTrace = stackTrace;
+            }
+
+            // Used to terminate the stack
+            RecordEntry()
+            {
+                this.hintString = null;
+                this.Next = null;
+                this.Pos = -1;
+                this.stackTrace = string.Empty;
+            }
+
+            public override string ToString()
+            {
+                var buf = new StringBuilder(2048);
+                if (this.hintString != null)
+                {
+                    buf.Append("\tHint: ").Append(this.hintString).Append(StringUtil.Newline);
+                }
+
+                // TODO: Use StackTrace class and support excludedMethods NETStandard2.0
+                // Append the stack trace.
+                buf.Append(this.stackTrace);
+                return buf.ToString();
+            }
         }
 
         class GCNotice

test/DotNetty.Buffers.Tests/AbstractCompositeByteBufferTests.cs

@@ -810,5 +810,22 @@ namespace DotNetty.Buffers.Tests
             ReferenceCountUtil.Release(buffer);
             Assert.Equal(0, buffer.ReferenceCount);
         }
+
+        [Fact]
+        public void AllocatorIsSameWhenCopy() => this.AllocatorIsSameWhenCopy(false);
+
+        [Fact]
+        public void AllocatorIsSameWhenCopyUsingIndexAndLength() => this.AllocatorIsSameWhenCopy(true);
+
+        void AllocatorIsSameWhenCopy(bool withIndexAndLength)
+        {
+            IByteBuffer buffer = this.NewBuffer(8);
+            buffer.WriteZero(4);
+            IByteBuffer copy = withIndexAndLength ? buffer.Copy(0, 4) : buffer.Copy();
+            Assert.Equal(buffer, copy);
+            Assert.Same(buffer.Allocator, copy.Allocator);
+            buffer.Release();
+            copy.Release();
+        }
     }
 }

test/DotNetty.Buffers.Tests/ConsolidationTest.cs

@@ -0,0 +1,65 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace DotNetty.Buffers.Tests
+{
+    using System.Text;
+    using Xunit;
+
+    using static Unpooled;
+
+    public class ConsolidationTest
+    {
+        [Fact]
+        public void ShouldWrapInSequence()
+        {
+            IByteBuffer currentBuffer = WrappedBuffer(WrappedBuffer(Encoding.ASCII.GetBytes("a")),
+                WrappedBuffer(Encoding.ASCII.GetBytes("=")));
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("1")),
+                WrappedBuffer(Encoding.ASCII.GetBytes("&")));
+
+            IByteBuffer copy = currentBuffer.Copy();
+            string s = copy.ToString(Encoding.ASCII);
+            Assert.Equal("a=1&", s);
+
+            currentBuffer.Release();
+            copy.Release();
+        }
+
+        [Fact]
+        public void ShouldConsolidationInSequence()
+        {
+            IByteBuffer currentBuffer = WrappedBuffer(WrappedBuffer(Encoding.ASCII.GetBytes("a")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("=")));
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("1")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("&")));
+
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("b")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("=")));
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("2")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("&")));
+
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("c")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("=")));
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("3")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("&")));
+
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("d")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("=")));
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("4")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("&")));
+
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("e")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("=")));
+            currentBuffer = WrappedBuffer(currentBuffer, WrappedBuffer(Encoding.ASCII.GetBytes("5")),
+                    WrappedBuffer(Encoding.ASCII.GetBytes("&")));
+
+            IByteBuffer copy = currentBuffer.Copy();
+            string s = copy.ToString(Encoding.ASCII);
+            Assert.Equal("a=1&b=2&c=3&d=4&e=5&", s);
+
+            currentBuffer.Release();
+            copy.Release();
+        }
+    }
+}

test/DotNetty.Buffers.Tests/EmptyByteBufferTests.cs

@@ -42,14 +42,5 @@ namespace DotNetty.Buffers.Tests
             Assert.Equal(0, empty.Array.Length);
             Assert.Equal(0, empty.ArrayOffset);
         }
-
-        [Fact]
-        public unsafe void MemoryAddress()
-        {
-            var empty = new EmptyByteBuffer(UnpooledByteBufferAllocator.Default);
-            Assert.False(empty.HasMemoryAddress);
-            byte* address;
-            Assert.Throws<NotSupportedException>(() => address = empty.MemoryAddress);
-        }
     }
 }