Closed yyjdelete closed 6 years ago
caozhiyuan
commented
6 years ago now when gc , gcnotice will be collect but buffer not write.
add a reference to gcnotice may can slove this problem @yyjdelete @StormHub .
https://github.com/caozhiyuan/DotNetty/blob/dev/src/DotNetty.Common/ResourceLeakDetector.cs#L81
yyjdelete
commented
6 years ago @caozhiyuan I make another patch, tested with DetectionLevel.Paranoid and Echo as you described in gitter, can you check if this works for you?
https://github.com/yyjdelete/DotNetty/commit/a67d13e86cd02f5fa74ea9cef19b4f33f5cfbc5f
@StormHub Should the key/value in gcNotificationMap be removed when Rearm is called? See CloseFinal.
StormHub
commented
6 years ago @yyjdelete No need to remove it for Rearm because it means that there is already an entry there. I still don't know what was the problem here.
caozhiyuan
commented
6 years ago @StormHub run this ` static void Main(string[] args) { DotNetty.Common.Internal.Logging.InternalLoggerFactory.DefaultFactory.AddConsole(); ResourceLeakDetector.Level = ResourceLeakDetector.DetectionLevel.Paranoid;
var logger = DotNetty.Common.Internal.Logging.InternalLoggerFactory.DefaultFactory.CreateLogger(typeof(Program));
var alloc = ByteBufferUtil.DefaultAllocator;
for (var i = 0; i < 2; ++i)
{
logger.LogInformation("start buf" + i);
var buf = alloc.Buffer();
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);
for (var j = 0; j < 4; ++j)
{
buf.WriteByte(i);
buf.WriteByte(j);
}
logger.LogInformation(ByteBufferUtil.HexDump(buf));
buf.Release();
logger.LogInformation("end buf" + i);
}
logger.LogInformation("The End");
Console.ReadLine();
}`
yyjdelete
commented
6 years ago @StormHub
Close not call GC.SuppressFinalize(GCNotice) and no check in CloseFinal to make sure object.ReferenceEquals(GCNotice.leak, this) before remove, so finalizer may be called after this buffer is added again and remove the new GCNotice. And the same thing for Rearm. See the missing assert in the second commit to tracking this. GCNotice will be collect right after Remove in Close with the modified CloseFinal.See https://github.com/Azure/DotNetty/compare/dev...yyjdelete:test-308
@caozhiyuan Can you check if it works for you? If it works, I will create an PR later.
StormHub
commented
6 years ago Let have a look at first.
StormHub
commented
6 years ago @yyjdelete I still cannot reproduce the problem here.
caozhiyuan
commented
6 years ago set ResourceLeakDetector.Level = ResourceLeakDetector.DetectionLevel.Paranoid; and i'm use netcoreapp2.0
StormHub
commented
6 years ago @caozhiyuan it is on Paranoid all the time and nothing unusual happened.
caozhiyuan
commented
6 years ago send you code , i have a look
yyjdelete
commented
6 years ago You can try increase the loop of i to 3 or more, 2 also not works for me.
StormHub
commented
6 years ago //When you doing the test, you might want to wait for finalizer thread to finish GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true); GC.WaitForPendingFinalizers(); // Empty finalizer queue
caozhiyuan
commented
6 years ago now i debug code add log, find buffer still in channeloutboundbuffer and not release , but gc tigger gcnotice finalize it report resource leak, so when buffer sent and release , this.Leak.Close(this.trackedByteBuf) return false
caozhiyuan
commented
6 years ago it may be no object ref gcnotice , and it can be gc finalized before buffer release and set null ?
caozhiyuan
commented
6 years ago i trace buffer when it reuse set a id to it
yyjdelete
commented
6 years ago StormHub
commented
6 years ago OK I think I know what happened here. The cached byte buffer gets reused before finalizer kicks in to remove the previous GCNotice entry first. When finalizer starts, it treats the current tracking records as leak for previously released byte buffer. The ConditionalWeakTable is keyed by byte buffer, the object.Equal returns true for reused byte buffer.
I have to look at how Netty does avoid this kind of situation before I come up with some fix.
BTW: I think #325 is caused by the same issue.
StormHub
commented
6 years ago @yyjdelete @caozhiyuan this worked, I need to give it a bit more thoughts
//DefaultResourceLeak class
public bool Close(object trackedObject)
{
if (this.owner.gcNotificationMap.TryGetValue(trackedObject, out GCNotice notice))
{
GC.SuppressFinalize(notice);
Debug.Assert(this.owner.gcNotificationMap.Remove(trackedObject));
Interlocked.Exchange(ref this.head, null);
return true;
}
return false;
}
nayato
commented
6 years ago fixed with #347
Seems it works well with 0.4.6
@StormHub A lot of things changed after 0.4.6, so don't what make the difference. Is an extra
GC.SuppressFinalize(GCNotice)is needed? (But not see in 0.4.6 either)Test Code
```cs using DotNetty.Buffers; using DotNetty.Common; using Microsoft.Extensions.Logging; using System; namespace ConsoleApp1 { class Program { static void Main(string[] args) { DotNetty.Common.Internal.Logging.InternalLoggerFactory.DefaultFactory.AddConsole(); ResourceLeakDetector.Level = ResourceLeakDetector.DetectionLevel.Paranoid; var logger = DotNetty.Common.Internal.Logging.InternalLoggerFactory.DefaultFactory.CreateLogger(typeof(Program)); var alloc = ByteBufferUtil.DefaultAllocator; for (var i = 0; i < 2; ++i) { logger.LogInformation("start buf" + i); var buf = alloc.Buffer(); GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true); for (var j = 0; j < 4; ++j) { buf.WriteByte(i); buf.WriteByte(j); } logger.LogInformation(ByteBufferUtil.HexDump(buf)); buf.Release(); logger.LogInformation("end buf" + i); } logger.LogInformation("The End"); Console.ReadLine(); } } } ```