bloomberg / bloomberg/memray

Make hook-reachable C++ independent from the process allocator

Open
#1,007 3 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.