Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sebastien-marichal opened this issue Aug 6, 2024 · 1 comment
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: New Rule Implementation for a rule that HAS been specified.

Comments

@sebastien-marichal
Copy link
Contributor

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.

@sebastien-marichal sebastien-marichal added Area: VB.NET VB.NET rules related issues. Area: C# C# rules related issues. Type: New Rule Implementation for a rule that HAS been specified. labels Aug 6, 2024
@jilles-sg
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: New Rule Implementation for a rule that HAS been specified.
Projects
None yet
Development

No branches or pull requests

2 participants