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

Tighten up some ArrayPool.Return usage #56229

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

stephentoub
Copy link
Member

Avoid potential problems if Return were to throw an exception after having already put the array back into the pool.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're guarding most of our usages like this, would having a Return(ref T[]? array) API make sense (letting that method null out the reference for the caller)?

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Jul 23, 2021

@MihaZupan Was thinking the same thing, but then we'd also have to teach people the difference between the two methods, including potentially even having analyzers for this. And if we ever do get a proper lifetime management feature in the language, it's all moot anyway.

Would a more straightforward alternative be to say "implementations of ArrayPool<T>.Return may check for usage errors and throw exceptions in those scenarios, but otherwise the methods must succeed"? If CERs were still a thing I imagine that's how we would have annotated this API, considering at its heart it is a manual resource management API.

Avoid potential problems if Return were to throw an exception after having already put the array back into the pool.
@stephentoub stephentoub merged commit 59a5610 into dotnet:main Jul 27, 2021
@stephentoub stephentoub deleted the tightenreturn branch July 27, 2021 11:40
@ghost ghost locked as resolved and limited conversation to collaborators Aug 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants