microsoft / microsoft/mimalloc
mem_region_s::commit is out of sync with VirtualAlloc on Windows
- Dominant language
- C
- Stars
- 13.4k
- Forks
- 1.2k
- Avg merge
- 4d 45m
- Merged PRs (30d)
- 13
Description
We found another issue with out of memory recovering, unfortunately we cannot provide reproducible example outside of our environment again, but after investigation we found out that mem_region_s::commit is out of sync with VirtualAlloc in out of memory state, and a non allocated/committed segment from mi_region_try_alloc (1, true, ...) is being returned.
Debug build crashes on
```
mimalloc/src/region.c(330): if (*commit) { ((uint8_t*)p)[0] = 0; }
```
Release build crashes in user code when returned pointer is dereferenced.
It could be fixed with this:
```
diff --git a/src/region.c b/src/region.c
index b6d0da3..0f24478 100644
--- a/src/region.c
+++ b/src/region.c
@@ -295,11 +295,12 @@ static void* mi_region_try_alloc(size_t blocks, bool* commit, bool* is_large, bo
// ensure commit
bool any_uncommitted;
mi_bitmap_claim(®ion->commit, 1, blocks, bit_idx, &any_uncommitted);
- if (any_uncommitted) {
+ if (any_uncommitted || (1 == blocks)) {
mi_assert_internal(!info.x.is_large);
bool commit_zero = false;
if (!_mi_mem_commit(p, blocks * MI_SEGMENT_SIZE, &commit_zero, tld)) {
```
but this fix negatively affected performance, so we ended up with another fix:
```
diff --git a/src/options.c b/src/options.c
index 9da3a9b..5ec9eef 100644
--- a/src/options.c
+++ b/src/options.c
@@ -66,7 +66,7 @@ static mi_option_desc_t options[_mi_option_last] =
// the following options are experimental and not all combinations make sense.
{ 1, UNINIT, MI_OPTION(eager_commit) }, // commit per segment directly (4MiB) (but see also `eager_commit_delay`)
- #if defined(_WIN32) || (MI_INTPTR_SIZE <= 4) // and other OS's without overcommit?
+ #if (MI_INTPTR_SIZE <= 4) // and other OS's without overcommit?
{ 0, UNINIT, MI_OPTION(eager_region_commit) },
{ 1, UNINIT, MI_OPTION(reset_decommits) }, // reset decommits memory
#else
```
where we got slightly improved performance and no crash.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/region.c at mi_region_try_alloc and the commit check around line 330, then compare its behavior with VirtualAlloc during out-of-memory conditions. Review src/options.c and the eager_region_commit setting; done means no unallocated or uncommitted segment is returned on Windows while avoiding the reported performance regression.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- operating-systems, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100