This repository was archived by the owner on Jan 23, 2023. It is now read-only.
[release/3.0] Do not use AllocateUninitializedArray in private array pools.#26366
Merged
Conversation
…et#26338) User may have extra expectations for the privatly owned array pools. For example there could be an expectation that array never contains negative numbers (since user never puts them there). Uninitialized allocations can break such expectations.
jkotas
approved these changes
Aug 26, 2019
|
/cc: @MeiChin-Tsai |
|
Stephen, do we now have test to catch this in the future? Just wondering how to prevent the same mistake in the future. Thanks. |
Member
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port #26338 to release/3.0.
Fixes https://github.com/dotnet/corefx/issues/40485
cc: @VSadov
Description
In 3.0 we added the GC.AllocateUninitializedArray API, which allocates an array but doesn't forcibly zero-init it (unless necessary for GC safety), and then we used it in a few places, including in the ArrayPool. However, we were over-aggressive about where we used it, and in addition to using it in the shared ArrayPool, we also used it in the ArrayPool type that's returned when ArrayPool.Create is used. With the shared pool, developers can't have any expectations about the state of the arrays returned to them when renting (because some other code, including from the core framework itself, may have returned a populated array), but with ArrayPool.Create, code can create isolated pools over which it has full control, and some code depends on getting back zero-inited arrays. This change just reverts the regression (#24504), going back to using
new T[...]instead ofGC.AllocateUninitializedArray(...)in that configurable/isolatable pool.Customer Impact
Non-zero'd arrays coming out of ArrayPool.Rent, which can then result in corruption for code that was expecting all elements to be zero.
Regression?
Yes, from .NET Core 2.2. This was reported by a customer while trying to upgrade to 3.0.
Risk
Very low. It's simply switching back three calls from
GC.AllocateUninitializedArray(...)to thenew T[...]instructions they were before.