mgravell / Pipelines.Sockets.Unofficial

.NET managed sockets wrapper using the new "Pipelines" API
Other
400 stars 51 forks source link

Memory Leak: BufferWriter Array Return Failure #76

Open DJGosnell opened 1 year ago

DJGosnell commented 1 year ago

Found a bug where the SequenceSegment<T>.Memory was being set to default prior to the RefCountedSegment.ReleaseImpl() method invocation. This has the effect of the implementation attempting to return default Memory<byte>.Empty array which is discarded by the pool. The GC obviously picks this up in the end, but it prevents the pool from reusing the memory.

The fix for this is changing the order of the setting of the memory to default and the ReleaseImpl invocation.

Added a test for this fix. I encountered one failure of BufferWriterTests.BufferWriterDoesNotLeak() but I could not reproduce the onetime failure. Unless I'm missing something, I don't see how this modification would cause the test failure.

mgravell commented 1 year ago

Interesting catch. Will read carefully asap