From 0e1aa1e42c843b48c060a762ff23b28d539b9899 Mon Sep 17 00:00:00 2001 From: robobun Date: Tue, 12 May 2026 15:27:27 +0000 Subject: [PATCH] win: release physical pages in vmDeallocatePhysicalPages + hintMemoryNotNeededSoon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows implementations of two memory-release primitives did not actually release physical pages: - bmalloc/VMAllocate.h::vmDeallocatePhysicalPages used VirtualAlloc with MEM_RESET, which marks contents as uninteresting but keeps physical frames and commit charge; it is not semantically equivalent to the POSIX madvise(MADV_DONTNEED) it shadows. - WTF/win/OSAllocatorWin.cpp::hintMemoryNotNeededSoon was an empty stub, so scavenger hints were silently dropped on Windows. Both now call DiscardVirtualMemory (Windows 8.1+, preserves the virtual mapping, frees physical frames, evicts from the working set). On older Windows 10 builds and on ranges spanning multiple reservations the call can fail, so they fall back to a per-reservation MEM_RESET loop followed by VirtualUnlock (which, on an unlocked page, evicts it from the working set) — the same pattern already used in libpas/pas_page_malloc.c. Fixes oven-sh/bun#30562. --- .../API/tests/WindowsVMDecommitTest.cpp | 108 ++++++++++++++++++ .../API/tests/WindowsVMDecommitTest.h | 39 +++++++ Source/JavaScriptCore/API/tests/testapi.c | 2 + Source/JavaScriptCore/shell/CMakeLists.txt | 1 + Source/WTF/wtf/win/OSAllocatorWin.cpp | 36 +++++- Source/bmalloc/bmalloc/VMAllocate.h | 37 +++++- 6 files changed, 219 insertions(+), 4 deletions(-) create mode 100644 Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.cpp create mode 100644 Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.h diff --git a/Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.cpp b/Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.cpp new file mode 100644 index 000000000000..5339f9c2f788 --- /dev/null +++ b/Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.cpp @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2025 Apple Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF + * THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "config.h" +#include "WindowsVMDecommitTest.h" + +#include +#include + +#if OS(WINDOWS) +#include +#include +#include +#endif + +WTF_ALLOW_UNSAFE_BUFFER_USAGE_BEGIN + +#if OS(WINDOWS) +static SIZE_T workingSetSize() +{ + PROCESS_MEMORY_COUNTERS counters; + counters.cb = sizeof(counters); + if (!GetProcessMemoryInfo(GetCurrentProcess(), &counters, sizeof(counters))) + return 0; + return counters.WorkingSetSize; +} +#endif + +void testWindowsVMDecommit() +{ +#if OS(WINDOWS) + printf("Testing Windows OSAllocator::hintMemoryNotNeededSoon releases pages (bun#30562)\n"); + + // 64 MiB is large enough to be unambiguous against background working-set + // noise (a few MiB of jitter between snapshots is normal) yet small enough + // to fit comfortably on any CI machine. + const size_t regionSize = 64 * 1024 * 1024; + + // Reserve + commit a single region so we know the exact page range and so + // VirtualQuery in the fallback path walks over reservations we control. + void* region = VirtualAlloc(nullptr, regionSize, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + RELEASE_ASSERT(region); + + // Touch every page to bring them into the working set. Without this, + // Windows lazy-commits — the pages won't show up in WorkingSetSize and + // there'd be nothing to observe being released. + SYSTEM_INFO systemInfo; + GetSystemInfo(&systemInfo); + const size_t pageSize = systemInfo.dwPageSize; + volatile unsigned char* bytes = static_cast(region); + for (size_t offset = 0; offset < regionSize; offset += pageSize) + bytes[offset] = static_cast(offset & 0xff); + + const SIZE_T before = workingSetSize(); + + // The call under test. Before the fix this was an empty stub — the + // working set would not change. After the fix it calls DiscardVirtualMemory + // (or the MEM_RESET + VirtualUnlock fallback) which evicts the pages. + WTF::OSAllocator::hintMemoryNotNeededSoon(region, regionSize); + + const SIZE_T after = workingSetSize(); + + VirtualFree(region, 0, MEM_RELEASE); + + printf(" working set before: %zu bytes\n", static_cast(before)); + printf(" working set after : %zu bytes\n", static_cast(after)); + + // Allow for the OS to keep a small residue in the working set. We require + // at least half the region to have been evicted — the previous empty-stub + // implementation would produce ~0 bytes of drop. + const SIZE_T minDrop = regionSize / 2; + if (before < after || before - after < minDrop) { + fprintf(stderr, "FAIL: hintMemoryNotNeededSoon did not release pages — " + "working set drop was %zu bytes, expected at least %zu bytes.\n", + before > after ? static_cast(before - after) : static_cast(0), + static_cast(minDrop)); + RELEASE_ASSERT_NOT_REACHED(); + } + printf("PASS: hintMemoryNotNeededSoon released %zu bytes from the working set\n", + static_cast(before - after)); +#else + printf("testWindowsVMDecommit: skipped (not Windows)\n"); +#endif +} + +WTF_ALLOW_UNSAFE_BUFFER_USAGE_END diff --git a/Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.h b/Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.h new file mode 100644 index 000000000000..00114b94ce07 --- /dev/null +++ b/Source/JavaScriptCore/API/tests/WindowsVMDecommitTest.h @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2025 Apple Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF + * THE POSSIBILITY OF SUCH DAMAGE. + */ + +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +/* Regression test for bun#30562: Windows vmDeallocatePhysicalPages and + * OSAllocator::hintMemoryNotNeededSoon must release physical pages, not + * just MEM_RESET. */ +void testWindowsVMDecommit(void); + +#ifdef __cplusplus +} /* extern "C" */ +#endif diff --git a/Source/JavaScriptCore/API/tests/testapi.c b/Source/JavaScriptCore/API/tests/testapi.c index 96a5697a92e7..4257a7886c43 100644 --- a/Source/JavaScriptCore/API/tests/testapi.c +++ b/Source/JavaScriptCore/API/tests/testapi.c @@ -71,6 +71,7 @@ #include "PingPongStackOverflowTest.h" #include "TypedArrayCTest.h" #include "VMManagerStopTheWorldTest.h" +#include "WindowsVMDecommitTest.h" WTF_ALLOW_UNSAFE_BUFFER_USAGE_BEGIN @@ -1625,6 +1626,7 @@ int main(int argc, char* argv[]) return failed; testCompareAndSwap(); + testWindowsVMDecommit(); startMultithreadedMultiVMExecutionTest(); // Test garbage collection with a fresh context diff --git a/Source/JavaScriptCore/shell/CMakeLists.txt b/Source/JavaScriptCore/shell/CMakeLists.txt index 0f9c28e1c83c..c559d189c326 100644 --- a/Source/JavaScriptCore/shell/CMakeLists.txt +++ b/Source/JavaScriptCore/shell/CMakeLists.txt @@ -37,6 +37,7 @@ if (DEVELOPER_MODE) ../API/tests/PingPongStackOverflowTest.cpp ../API/tests/TypedArrayCTest.cpp ../API/tests/VMManagerStopTheWorldTest.cpp + ../API/tests/WindowsVMDecommitTest.cpp ../API/tests/testapi.cpp ) set(testapi_C_SOURCES diff --git a/Source/WTF/wtf/win/OSAllocatorWin.cpp b/Source/WTF/wtf/win/OSAllocatorWin.cpp index c25a38ac8972..32e969d93d7f 100644 --- a/Source/WTF/wtf/win/OSAllocatorWin.cpp +++ b/Source/WTF/wtf/win/OSAllocatorWin.cpp @@ -26,6 +26,7 @@ #include "config.h" #include +#include #include #include #include @@ -127,8 +128,41 @@ void OSAllocator::releaseDecommitted(void* address, size_t bytes, unsigned) CRASH(); } -void OSAllocator::hintMemoryNotNeededSoon(void*, size_t) +void OSAllocator::hintMemoryNotNeededSoon(void* address, size_t bytes) { + // Windows equivalent of madvise(MADV_DONTNEED). The pages must remain + // accessible (the POSIX version leaves the mapping in place and just + // drops the physical backing), so we can't MEM_DECOMMIT here. + // + // Previously this was an empty stub, which meant scavenger hints were + // silently dropped on Windows — long-running processes accumulated + // hundreds of MB of committed-but-zero pages that the OS never + // reclaimed (bun#30562). + // + // DiscardVirtualMemory releases the physical frames immediately and + // removes the pages from the working set while keeping the virtual + // mapping committed. On older Windows 10 builds and on ranges spanning + // multiple reservations it can fail, so we fall back to a + // per-reservation MEM_RESET loop + VirtualUnlock, which has the same + // effect with lazy reclaim. This mirrors the pattern already used in + // libpas (pas_page_malloc.c). + if (!bytes) + return; + if (DiscardVirtualMemory(address, bytes)) { + size_t totalSeen = 0; + void* currentPtr = address; + while (totalSeen < bytes) { + MEMORY_BASIC_INFORMATION memInfo; + SIZE_T queried = VirtualQuery(currentPtr, &memInfo, sizeof(memInfo)); + if (!queried || !memInfo.RegionSize) + return; + size_t chunkSize = std::min(memInfo.RegionSize, bytes - totalSeen); + VirtualAlloc(currentPtr, chunkSize, MEM_RESET, PAGE_READWRITE); + currentPtr = reinterpret_cast(reinterpret_cast(currentPtr) + memInfo.RegionSize); + totalSeen += memInfo.RegionSize; + } + VirtualUnlock(address, bytes); + } } bool OSAllocator::tryProtect(void* address, size_t bytes, bool readable, bool writable) diff --git a/Source/bmalloc/bmalloc/VMAllocate.h b/Source/bmalloc/bmalloc/VMAllocate.h index 036636ceec15..be4436bb2985 100644 --- a/Source/bmalloc/bmalloc/VMAllocate.h +++ b/Source/bmalloc/bmalloc/VMAllocate.h @@ -429,10 +429,41 @@ inline void vmZeroAndPurge(void* p, size_t vmSize, VMTag usage) inline void vmDeallocatePhysicalPages(void* p, size_t vmSize) { + // Windows equivalent of madvise(MADV_DONTNEED): release physical backing + // while keeping the virtual range committed and accessible. The paired + // vmAllocatePhysicalPages() re-commits on reuse. + // + // MEM_RESET alone is NOT sufficient — it marks contents as uninteresting + // but does not release physical frames or remove pages from the working + // set, so RSS and commit charge grow unboundedly on long-running + // processes (bun#30562). + // + // DiscardVirtualMemory frees the physical pages immediately and removes + // them from the working set. On older Windows 10 builds and on ranges + // spanning multiple reservations it can fail, so we fall back to a + // per-reservation MEM_RESET loop + VirtualUnlock, which evicts the pages + // from the working set with lazy reclaim. This mirrors the pattern + // already used in libpas (pas_page_malloc.c). vmValidatePhysical(p, vmSize); - bool writable = true; - bool executable = true; - VirtualAlloc(p, vmSize, MEM_RESET, protection(writable, executable)); + if (DiscardVirtualMemory(p, vmSize)) { + bool writable = true; + bool executable = true; + size_t totalSeen = 0; + void* currentPtr = p; + while (totalSeen < vmSize) { + MEMORY_BASIC_INFORMATION memInfo; + SIZE_T bytes = VirtualQuery(currentPtr, &memInfo, sizeof(memInfo)); + RELEASE_BASSERT(bytes == sizeof(memInfo)); + RELEASE_BASSERT(memInfo.RegionSize > 0); + size_t chunkSize = std::min(memInfo.RegionSize, vmSize - totalSeen); + VirtualAlloc(currentPtr, chunkSize, MEM_RESET, protection(writable, executable)); + currentPtr = reinterpret_cast(reinterpret_cast(currentPtr) + memInfo.RegionSize); + totalSeen += memInfo.RegionSize; + } + // VirtualUnlock of unlocked pages evicts them from the working set, + // which MEM_RESET alone does not do. + VirtualUnlock(p, vmSize); + } } inline void vmAllocatePhysicalPages(void* p, size_t vmSize)