-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix(windows): implement bmalloc SystemHeap using mimalloc to fix GC crashes #28249
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,286 @@ | ||
| /** | ||
| * Working SystemHeap implementation for Windows using mimalloc. | ||
| * | ||
| * bmalloc's SystemHeap is unimplemented on Windows — all methods crash with | ||
| * RELEASE_BASSERT_NOT_REACHED. This file provides a functional replacement | ||
| * that uses mimalloc for allocations. | ||
| * | ||
| * Combined with setting the "Malloc" environment variable at process startup, | ||
| * this causes bmalloc/libpas to redirect all allocations through SystemHeap | ||
| * instead of using libpas directly, avoiding GC crashes on Windows. | ||
| * | ||
| * See: https://github.com/oven-sh/bun/issues/22349 | ||
| * https://github.com/oven-sh/bun/issues/21569 | ||
| * | ||
| * How it works: | ||
| * 1. Bun sets Malloc=1 env var early in main() (see src/main.zig) | ||
| * 2. bmalloc's Environment reads Malloc=1 and enables SystemHeap | ||
| * 3. All bmalloc allocations route through SystemHeap | ||
| * 4. This file provides SystemHeap using mimalloc instead of crashing | ||
| * | ||
| * Link-time override: | ||
| * This object file defines the same symbols as bmalloc.lib's | ||
| * SystemHeap.cpp.o. Since object files are processed before static | ||
| * libraries, the linker uses these definitions and skips bmalloc.lib's | ||
| * unimplemented stubs. | ||
| */ | ||
|
|
||
| #include <bmalloc/BPlatform.h> | ||
|
|
||
| #if BOS(WINDOWS) | ||
|
|
||
| // We are providing the implementation of bmalloc's SystemHeap, so we need | ||
| // BEXPORT to resolve to __declspec(dllexport) rather than dllimport. | ||
| #define BUILDING_bmalloc | ||
|
|
||
| #include <bmalloc/SystemHeap.h> | ||
| #include <bmalloc/Algorithm.h> | ||
| #include <bmalloc/BAssert.h> | ||
| #include <bmalloc/VMAllocate.h> | ||
| #include "mimalloc.h" | ||
|
|
||
| #if BENABLE(LIBPAS) | ||
| #include <bmalloc/pas_system_heap.h> | ||
| #endif | ||
|
|
||
| namespace bmalloc { | ||
|
|
||
| SystemHeap* systemHeapCache { nullptr }; | ||
|
|
||
| BALLOW_DEPRECATED_DECLARATIONS_BEGIN | ||
| DEFINE_STATIC_PER_PROCESS_STORAGE(SystemHeap); | ||
| BALLOW_DEPRECATED_DECLARATIONS_END | ||
|
|
||
| SystemHeap::SystemHeap(const LockHolder&) | ||
| : m_pageSize(vmPageSize()) | ||
| { | ||
| } | ||
|
|
||
| void* SystemHeap::malloc(size_t size, FailureAction action) | ||
| { | ||
| void* result = mi_malloc(size); | ||
| RELEASE_BASSERT(action == FailureAction::ReturnNull || result); | ||
| return result; | ||
| } | ||
|
|
||
| void* SystemHeap::memalign(size_t alignment, size_t size, FailureAction action) | ||
| { | ||
| void* result = mi_malloc_aligned(size, alignment); | ||
| RELEASE_BASSERT(action == FailureAction::ReturnNull || result); | ||
| return result; | ||
| } | ||
|
|
||
| void* SystemHeap::realloc(void* object, size_t size, FailureAction action) | ||
| { | ||
| void* result = mi_realloc(object, size); | ||
| RELEASE_BASSERT(action == FailureAction::ReturnNull || result); | ||
| return result; | ||
| } | ||
|
|
||
| void SystemHeap::free(void* object) | ||
| { | ||
| mi_free(object); | ||
| } | ||
|
|
||
| void SystemHeap::scavenge() | ||
| { | ||
| mi_collect(false); | ||
| } | ||
|
|
||
| void SystemHeap::dump() | ||
| { | ||
| } | ||
|
|
||
| void* SystemHeap::memalignLarge(size_t alignment, size_t size) | ||
| { | ||
| alignment = roundUpToMultipleOf(m_pageSize, alignment); | ||
| size = roundUpToMultipleOf(m_pageSize, size); | ||
| void* result = tryVMAllocate(alignment, size); | ||
| if (!result) | ||
| return nullptr; | ||
| { | ||
| LockHolder locker(mutex()); | ||
| m_sizeMap[result] = size; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| void SystemHeap::freeLarge(void* base) | ||
| { | ||
| if (!base) | ||
| return; | ||
|
|
||
| size_t size; | ||
| { | ||
| LockHolder locker(mutex()); | ||
| size = m_sizeMap[base]; | ||
| size_t numErased = m_sizeMap.erase(base); | ||
| RELEASE_BASSERT(numErased == 1); | ||
|
Comment on lines
+108
to
+118
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
header="$(fd 'SystemHeap\.h$' . | head -n1)"
test -n "$header"
echo "== inspecting ${header} =="
rg -n -C5 '\bclass SystemHeap\b|\bm_sizeMap\b|\btake\s*\(|\bfind\s*\(|\berase\s*\(' "$header"Repository: oven-sh/bun Length of output: 37 🏁 Script executed: #!/bin/bash
set -euo pipefail
# First, let's check the .cpp file directly
cpp_file="src/bun.js/bindings/BmallocSystemHeapOverride.cpp"
if [ ! -f "$cpp_file" ]; then
echo "File not found at expected location"
exit 1
fi
echo "=== Examining the .cpp file for includes and m_sizeMap usage ==="
head -n 150 "$cpp_file"Repository: oven-sh/bun Length of output: 4000 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Search for SystemHeap.h in the repository
echo "=== Looking for SystemHeap.h ==="
find . -name "SystemHeap.h" -type f 2>/dev/null | head -20
# Also search for bmalloc headers
echo -e "\n=== Looking for bmalloc directory ==="
find . -type d -name "bmalloc" 2>/dev/null | head -10Repository: oven-sh/bun Length of output: 126 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Search for any bmalloc references or comments
echo "=== Searching for bmalloc references in the repo ==="
rg -l "bmalloc" --type-list 2>/dev/null | head -10
# Look for any comments or documentation mentioning SystemHeap
echo -e "\n=== Searching for SystemHeap references ==="
rg "SystemHeap" --type-list 2>/dev/null | head -5
# Search within the cpp file for m_sizeMap or map container hints
echo -e "\n=== Searching for map/HashMap patterns in bindings directory ==="
fd "\.h$|\.cpp$" src/bun.js/bindings/ | xargs rg "sizeMap|HashMap|map<" 2>/dev/null | head -20Repository: oven-sh/bun Length of output: 107 🌐 Web query:
💡 Result: In WebKit’s bmalloc::SystemHeap,
To look up an entry, use auto it = m_sizeMap.find(base);
if (it != m_sizeMap.end()) {
size_t size = it->second;
}(Internally, the code also uses Sources: Use
Replace with: auto it = m_sizeMap.find(base);
RELEASE_BASSERT(it != m_sizeMap.end());
size_t size = it->second;
m_sizeMap.erase(it);This ensures the allocation was actually tracked before proceeding. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| vmDeallocate(base, size); | ||
| } | ||
|
|
||
| SystemHeap* SystemHeap::tryGetSlow() | ||
| { | ||
| SystemHeap* result; | ||
| if (Environment::get()->isSystemHeapEnabled()) { | ||
| systemHeapCache = SystemHeap::get(); | ||
| result = systemHeapCache; | ||
| } else { | ||
| systemHeapCache = systemHeapDisabled(); | ||
| result = nullptr; | ||
| } | ||
| RELEASE_BASSERT(systemHeapCache); | ||
| return result; | ||
| } | ||
|
|
||
| } // namespace bmalloc | ||
|
|
||
| #if BENABLE(LIBPAS) | ||
|
|
||
| #if BUSE(LIBPAS) | ||
|
|
||
| using namespace bmalloc; | ||
|
|
||
| bool pas_system_heap_is_enabled(pas_heap_config_kind kind) | ||
| { | ||
| switch (kind) { | ||
| case pas_heap_config_kind_bmalloc: | ||
| return !!SystemHeap::tryGet(); | ||
| case pas_heap_config_kind_jit: | ||
| case pas_heap_config_kind_pas_utility: | ||
| return false; | ||
| default: | ||
| BCRASH(); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| bool pas_system_heap_should_supplant_bmalloc(pas_heap_config_kind kind) | ||
| { | ||
| SystemHeap* heap; | ||
| switch (kind) { | ||
| case pas_heap_config_kind_bmalloc: | ||
| heap = SystemHeap::tryGet(); | ||
| if (!heap) | ||
| return false; | ||
| return heap->shouldSupplantBmalloc(); | ||
| case pas_heap_config_kind_jit: | ||
| case pas_heap_config_kind_pas_utility: | ||
| return false; | ||
| default: | ||
| BCRASH(); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| void* pas_system_heap_malloc(size_t size) | ||
| { | ||
| return SystemHeap::getExisting()->malloc(size, FailureAction::ReturnNull); | ||
| } | ||
|
|
||
| void* pas_system_heap_memalign(size_t alignment, size_t size) | ||
| { | ||
| return SystemHeap::getExisting()->memalign(alignment, size, FailureAction::ReturnNull); | ||
| } | ||
|
|
||
| void* pas_system_heap_realloc(void* ptr, size_t size) | ||
| { | ||
| return SystemHeap::getExisting()->realloc(ptr, size, FailureAction::ReturnNull); | ||
| } | ||
|
|
||
| void pas_system_heap_free(void* ptr) | ||
| { | ||
| SystemHeap::getExisting()->free(ptr); | ||
| } | ||
|
|
||
| void* pas_system_heap_malloc_compact(size_t size) | ||
| { | ||
| return SystemHeap::getExisting()->malloc(size, FailureAction::ReturnNull); | ||
| } | ||
|
|
||
| void* pas_system_heap_memalign_compact(size_t alignment, size_t size) | ||
| { | ||
| return SystemHeap::getExisting()->memalign(alignment, size, FailureAction::ReturnNull); | ||
| } | ||
|
|
||
| void* pas_system_heap_realloc_compact(void* ptr, size_t size) | ||
| { | ||
| return SystemHeap::getExisting()->realloc(ptr, size, FailureAction::ReturnNull); | ||
| } | ||
|
|
||
| #else // !BUSE(LIBPAS) | ||
|
|
||
| bool pas_system_heap_is_enabled(pas_heap_config_kind kind) | ||
| { | ||
| BUNUSED_PARAM(kind); | ||
| return false; | ||
| } | ||
|
|
||
| bool pas_system_heap_should_supplant_bmalloc(pas_heap_config_kind kind) | ||
| { | ||
| BUNUSED_PARAM(kind); | ||
| return false; | ||
| } | ||
|
|
||
| void* pas_system_heap_malloc(size_t size) | ||
| { | ||
| BUNUSED_PARAM(size); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| return nullptr; | ||
| } | ||
|
|
||
| void* pas_system_heap_memalign(size_t alignment, size_t size) | ||
| { | ||
| BUNUSED_PARAM(size); | ||
| BUNUSED_PARAM(alignment); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| return nullptr; | ||
| } | ||
|
|
||
| void* pas_system_heap_realloc(void* ptr, size_t size) | ||
| { | ||
| BUNUSED_PARAM(ptr); | ||
| BUNUSED_PARAM(size); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| return nullptr; | ||
| } | ||
|
|
||
| void* pas_system_heap_malloc_compact(size_t size) | ||
| { | ||
| BUNUSED_PARAM(size); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| return nullptr; | ||
| } | ||
|
|
||
| void* pas_system_heap_memalign_compact(size_t alignment, size_t size) | ||
| { | ||
| BUNUSED_PARAM(size); | ||
| BUNUSED_PARAM(alignment); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| return nullptr; | ||
| } | ||
|
|
||
| void* pas_system_heap_realloc_compact(void* ptr, size_t size) | ||
| { | ||
| BUNUSED_PARAM(ptr); | ||
| BUNUSED_PARAM(size); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| return nullptr; | ||
| } | ||
|
|
||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wmissing-noreturn" | ||
| void pas_system_heap_free(void* ptr) | ||
| { | ||
| BUNUSED_PARAM(ptr); | ||
| RELEASE_BASSERT_NOT_REACHED(); | ||
| } | ||
| #pragma clang diagnostic pop | ||
|
|
||
| #endif // BUSE(LIBPAS) | ||
|
|
||
| #endif // BENABLE(LIBPAS) | ||
|
|
||
| #endif // BOS(WINDOWS) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,20 @@ pub extern "c" var environ: ?*anyopaque; | |
| pub fn main() void { | ||
| _bun.crash_handler.init(); | ||
|
|
||
| // Force bmalloc to use SystemHeap (mimalloc) instead of libpas on Windows. | ||
| // bmalloc's libpas allocator causes GC crashes under memory pressure on | ||
| // Windows because SystemHeap is unimplemented (all methods assert-crash). | ||
| // Setting "Malloc" causes bmalloc::Environment to route all allocations | ||
| // through SystemHeap, which we override with mimalloc | ||
| // (see BmallocSystemHeapOverride.cpp). Must be set before any bmalloc | ||
| // allocation triggers Environment initialization. | ||
| // See: https://github.com/oven-sh/bun/issues/22349 | ||
| if (Environment.isWindows) { | ||
| // _putenv returns 0 on success, -1 on failure. Failure here means | ||
| // bmalloc will use unimplemented libpas and crash later, so assert. | ||
| _bun.assert(_bun.c._putenv("Malloc=1") == 0); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
+32
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The Extended reasoning...What the bug isThis PR adds The specific code pathIn Why existing code doesn't prevent itThe two entry points are completely separate files with duplicated initialization logic. Changes to one are not automatically reflected in the other. The PR description and commit only modified ImpactWithout Step-by-step proof
How to fixAdd |
||
|
|
||
| if (Environment.isPosix) { | ||
| var act: std.posix.Sigaction = .{ | ||
| .handler = .{ .handler = std.posix.SIG.IGN }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 106
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 110
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 8808
🌐 Web query:
site:raw.githubusercontent.com WebKit SystemHeap.cpp pas_system_heap_malloc PAS_PROFILE💡 Result:
In WebKit’s
Source/bmalloc/bmalloc/SystemHeap.cpp(commit1034e0634fd9bc7f1413cea076d0536b0e7cf87c), the libpas glue definespas_system_heap_malloc()and instruments it withPAS_PROFILEbefore delegating toSystemHeap:pas_system_heap_malloc(size_t size):auto systemHeap = SystemHeap::getExisting();PAS_PROFILE(SYSTEM_HEAP_ALLOCATION, systemHeap, size, 0, pas_non_compact_allocation_mode);systemHeap->malloc(size, FailureAction::ReturnNull);[1]The corresponding declarations live in
Source/bmalloc/libpas/src/libpas/pas_system_heap.h(same commit), declaringpas_system_heap_malloc,pas_system_heap_memalign, etc., asBEXPORT extern ...whenPAS_BMALLOCis enabled. [2]Sources: [1], [2]
🌐 Web query:
site:raw.githubusercontent.com WebKit SystemHeap.cpp PAS_MTE_HANDLE💡 Result:
In WebKit’s repo,
PAS_MTE_HANDLEappears inSource/bmalloc/bmalloc/SystemHeap.cpp(thepas_system_heap_*allocation wrappers), e.g. these functions call it:pas_system_heap_mallocpas_system_heap_memalignpas_system_heap_reallocpas_system_heap_malloc_compactpas_system_heap_memalign_compactpas_system_heap_realloc_compact[1]Raw files:
Apply the same pattern to
memalign,realloc, and their*_compactvariants.🤖 Prompt for AI Agents