-
Notifications
You must be signed in to change notification settings - Fork 49
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
Unbounded Memory Leak #106
Comments
I updated the report to show a much more minimal repro. I have determined that something in the compiler toolchain can elide the allocations that cause this issue when optimizing if I inline everything. Thus my repro uses a reduce optimization level. The explicit drop calls are needed: otherwise not leak is produced. The sizes do matter. For example if they are ~10x lower there is no leak. pub fn leak_test() {
// This leaks when using wee_alloc
let a = Box::new([0; 80000]);
let b = Box::new([0; 80000]);
drop(b);
drop(a);
} |
After more testing, size 8190 (and less) does not have the issue, and 8191 (and higher) do. This is just below 2^13, so I suspect the issue is when the allocation + some overhead exceeds 2^13, the memory is not reused. |
This reproduces in all versions of wee_alloc that can compile for me: current master, 0.4.5, 0.4.4, 0.4.3 |
8190 and 8191 byte allocations (sum: 16381): leaks 16281 and 100 byte allocations (sum: 16381): does not leak Looks like it leaks if the two allocations sum to 16381 (~2^14) or more, and both exceed some minimum size threshold, it leaks, except I found these cases which doesn't leak, so thats not 100%. 16081 and 2188 byte allocations (sum: 18269): does not leak |
In case that is useful for others, I also encountered this bug while compressing images using |
I have confirmed your repro does leak memory. My repro was from a deflate implementation. Png uses deflate for its compression, so I expect its nearly identical to my repro, except yours is native code instead of wasm. I prefer the no-wasm repro, so I have made a fork of your example based on my simplified 2 allocations version: https://github.com/CraigMacomber/wee_alloc_leak_png |
Here is an updated repro. My best guess for whats happening is retrieving space for the first allocation from the free list (which hits the edge case of the space being large enough, but not having enough extra space in the allocation to split the node in the free list) is incorrectly nulling out the head of the free list, leaking the other allocations remaining in the free list and causing the second allocation to have to allocate new memory. use std::alloc::GlobalAlloc;
extern crate wee_alloc;
// Use `wee_alloc` as the global allocator.
#[global_allocator]
static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT;
fn main() {
loop {
let l1 = core::alloc::Layout::from_size_align(85196, 1).unwrap();
let l2 = core::alloc::Layout::from_size_align(80000, 1).unwrap();
unsafe {
// Allocate first block: this one gets reused from the free list,
// but also causes the head of the free list to be set to null when it should still have space in it.
let x1 = ALLOC.alloc(l1);
// Allocate second block: this one does not reuse space from the free list (which seems like a bug).
// At this point wee_alloc's free list's head (as used in alloc_first_fit) is null.
// This results in a new allocation.
let x2 = ALLOC.alloc(l2);
println!("{x1:p}, {x2:p}");
ALLOC.dealloc(x1, l1);
ALLOC.dealloc(x2, l2);
}
}
} The call stack when the head of the free list is set to null (when allocating the first allocation in the loop on the second iteration of the loop, from the second node in the free list). In "try_alloc" previous is the overall head of the list, and that line sets it to null. My understanding is that this causes it to forget about the first item in the free list (which was too small for this allocation, but would fix the next one).
|
A different related repro (I have updated the example linked in the top post to include this): fn main() {
static WEE: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT;
loop {
// These are magic sizes for when size categories are disabled.
// If either of these sizes is decreased, it no longer behaves the same (no longer gets new allocations for both every time).
let l1 = core::alloc::Layout::from_size_align(2033, 1).unwrap(); // Rounds up to 255 words.
let l2 = core::alloc::Layout::from_size_align(2025, 1).unwrap(); // Rounds up to 254 words.
unsafe {
// Allocate first block: this one gets space that was use by the second allocation on the last iteration, but starting at an address 8 bytes lower.
let x1 = WEE.alloc(l1);
// Allocate second block: this one does not reuse space from the free list (which seems like a bug).
// At this point wee_alloc's free list seems to only contain one item which does not fit.
let x2 = WEE.alloc(l2);
println!("{x1:p}, {x2:p}");
WEE.dealloc(x1, l1);
WEE.dealloc(x2, l2);
}
}
} |
I think this closes #305. rustwasm/wee_alloc#106 is the most likely issue.
I think this closes #305. rustwasm/wee_alloc#106 is the most likely issue.
The app crashes with memory corruption and consumes excessive amounts of memory when wee_alloc is enabled. See: rustwasm/wee_alloc#106
is related to rustwasm/wee_alloc#106
`wee_alloc` crate is currently unmaintained and has a few open issues. In particular, one of these issue is an unbounded memeroy leak. As such, we stop considering this crate as production-ready and switch to the default Rust standard allocator in newer NPM packages. - rustwasm/wee_alloc#106 - rustwasm/wee_alloc#107 - https://rustsec.org/advisories/RUSTSEC-2022-0054.html Issue: ARC-98
`wee_alloc` crate is currently unmaintained and has a few open issues. In particular, one of these issue is an unbounded memeroy leak. As such, we stop considering this crate as production-ready and switch to the default Rust standard allocator in newer NPM packages. - rustwasm/wee_alloc#106 - rustwasm/wee_alloc#107 - https://rustsec.org/advisories/RUSTSEC-2022-0054.html Issue: ARC-98
Repeatedly calling this library causes unbounded memory growth. Wee-alloc has known memory leak issues. See rustwasm/wee_alloc#106 Rust's standard library allocator can be used instead.
Memory leak was caused by rustwasm/wee_alloc#106. This PR fixes the leak by removes lightweight allocator I have also added a test, which is now disabled, that hashes 17GB in a loop to verify expected memory use. Test is disabled because it takes a long time to run.
Crate has unresolved major issues such as memory leaks (rustwasm/wee_alloc#106). wee_alloc's last commit at the moment was made shy of 3 years ago, and the latest release is over 4 years old.
Crate has major unresolved issues such as memory leaks (rustwasm/wee_alloc#106). wee_alloc's last commit at the moment was made shy of 3 years ago, and the latest release is over 4 years old.
Crate has major unresolved issues such as memory leaks (rustwasm/wee_alloc#106). Its last commit at the moment was made shy of 3 years ago, and the latest release is over 4 years old. I see no reason to keep using it in light of this.
Crate has major unresolved issues such as memory leaks (rustwasm/wee_alloc#106). Its last commit at the moment was made shy of 3 years ago, and the latest release is over 4 years old. I see no reason to keep using it in light of this.
Describe the Bug
Making two large allocations, then dropping them in the order they were allocated leaks memory in wee_alloc, but not in default allocator.
Steps to Reproduce
Native code
cargo run
cargo run
Wasm
npm run serve
(runs wasm-pack, npm install and webpack dev server)Expected Behavior
Second time allocations are made uses free list.
Actual Behavior
Repeated allocations grow heap infinitely.
Additional Context
This seems similar to an issue mentioned in #105
Heap size does not increase if using default allocator.
Rust source for example is just:
The text was updated successfully, but these errors were encountered: