SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
773 stars 226 forks source link

New Rule Idea: Buffers rented from buffer pools should be returned #9607

Open sebastien-marichal opened 1 month ago

sebastien-marichal commented 1 month ago

Buffers allocated using buffer pools such as ArrayPool should be returned after usage.

Noncompliant example

void Method(Stream stream)
{
    var buffer = ArrayPool<byte>.Shared.Rent(512); // Noncompliant buffer is not returned
    var len = stream.Read(buffer, 0, 512);
    // ...
}

Compliant example

void Method(Stream stream)
{
    var buffer = ArrayPool<byte>.Shared.Rent(512); // Compliant
    var len = stream.Read(buffer, 0, 512);
    // ...
    ArrayPool<byte>.Shared.Return(buffer);
}

Notes

MemorePool does not have a Return and needs more investigation.

jilles-sg commented 6 days ago

Left quiet here is that the ArrayPool<byte>.Shared.Return() call is not in a finally block. This is not wrong but how it should be (assuming that an exception is expected to be rare), since it reduces overhead. Not returning arrays to ArrayPool<T>.Shared does not cause leaks, but provably never returning a particular array is of course wrong.

MemoryPool<T>.Rent() returns an object that implements IDisposable and should be disposed of. It should be assumed that not disposing may cause leaks, so using or finally should be used. Given that the returned object tends to be a new allocation (albeit a small one), there seems little reason to micro-optimize away a finally block.