Fix for assert failure in distribute free regions#66657
Conversation
…mong the highest free regions, and so gets moved to the decommit list again by region_allocator::move_highest_free_regions. The fix is simply to check whether the region is already on the to_free_list and not move it or count it if it is.
|
Tagging subscribers to this area: @dotnet/gc Issue DetailsI had failed to take into account the case where we move a free region to the decommit list due to age, but it is among the free regions with the highest address, and so is encountered by move_highest_free_regions as well. What happens then is that move_highest_free_regions removes it from the decommit list, and puts it on the decommit list again. No harm done, except that we didn't move as many regions to the decommit list as planned. Fix is to check in move_highest_free_regions whether a region is already on the decommit list, and skip it in this case. I preferred this to loosening the assert because it makes sure we actually decommit as much as planned.
|
* Fix the case where a region that was decommitted due to age is also among the highest free regions, and so gets moved to the decommit list again by region_allocator::move_highest_free_regions. The fix is simply to check whether the region is already on the to_free_list and not move it or count it if it is. * Fix typo. * Fix logic in move_highest_free_regions.
ghost
locked as resolved and limited conversation to collaborators
3 participants
I had failed to take into account the case where we move a free region to the decommit list due to age, but it is among the free regions with the highest address, and so is encountered by move_highest_free_regions as well.
What happens then is that move_highest_free_regions removes it from the decommit list, and puts it on the decommit list again. No harm done, except that we didn't move as many regions to the decommit list as planned.
Fix is to check in move_highest_free_regions whether a region is already on the decommit list, and skip it in this case. I preferred this to loosening the assert because it makes sure we actually decommit as much as planned.
Fixes: #66601