Make hook-reachable C++ independent from the process allocator
- Dominant language
- Python
- Stars
- 15.2k
- Forks
- 461
- Avg merge
- 5d 3h
- Merged PRs (30d)
- 10
Description
The allocator hooks can allocate through the allocator they are currently intercepting. This normally works, but it can deadlock if the hook was entered while malloc/jemalloc/tcmalloc/etc. was holding one of its internal locks.
`libhugetlbfs` is a good way to expose it, but this is not really a huge-pages-specific bug. On older glibc, malloc can call `MORECORE` while the arena is locked ([glibc source](https://github.com/bminor/glibc/blob/glibc-2.33/malloc/malloc.c)). `libhugetlbfs` replaces that with an implementation which calls `mmap` ([source](https://github.com/libhugetlbfs/libhugetlbfs/blob/master/morecore.c)), so we can get:
```
malloc -> allocator lock -> mmap -> Memray mmap hook
-> C++ allocation -> malloc -> same lock forever
```
We have already seen the same shape with jemalloc in [#669](https://github.com/bloomberg/memray/issues/669): jemalloc called `mmap` while creating an arena and Memray created a `std::vector` from the mmap hook, which went back into jemalloc. Fixing that one vector does not prevent another string, map, TLS object, exception, or library call from reintroducing the problem.
A recursion guard does not solve this. It can stop us recording the nested allocation, but the real nested `malloc` still tries to take the lock which is already held.
The invariant should be that Memray-owned dynamic C++ storage reachable from an allocation/deallocation hook never uses the process allocator. This must not change the current profiling behavior: in particular, native stacks for `mmap` events stay enabled.
There are several small implementations we can borrow from: allocator-aware STL aliases backed by a private pool, like the [LLVM sanitizer allocator](https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h), [gperftools LowLevelAlloc](https://github.com/gperftools/gperftools/blob/master/src/base/low_level_alloc.cc), or [Perfetto's unhooked allocator](https://github.com/google/perfetto/blob/main/src/profiling/memory/unhooked_allocator.h). We should not globally replace `new`/`delete`, because allocations crossing shared-library boundaries can then be freed by the wrong heap.
We also need a hostile regression test which holds a fake allocator lock across `mmap`/`munmap` and fails if Memray-owned hook code calls `malloc`, `calloc`, `realloc`, or `free`. That gives us one generic check instead of fixing this allocator by allocator or container by container.
Libunwind can introduce the same inversion through the loader lock ([gperftools example](https://github.com/gperftools/gperftools/issues/1159)). Its [`unw_set_iterate_phdr_function`](https://github.com/libunwind/libunwind/blob/master/doc/unw_set_iterate_phdr_function.man) API lets us give it an immutable program-header snapshot, so native `mmap` stacks can stay enabled without calling `dl_iterate_phdr` from the allocation hook.
Contributor guide
Research direction
Start by tracing the allocation/deallocation hooks and the native mmap stack path described in the issue, then review libunwind's unw_set_iterate_phdr_function API. Add a hostile regression test that holds a fake allocator lock across mmap/munmap and detects malloc, calloc, realloc, or free from hook code. Done means hook-reachable C++ storage avoids the process allocator while native mmap stacks remain enabled.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- devtools, performance, tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100