Support posix_memalign and aligned_alloc in the native memory tracker #1130
Conversation
| } | ||
|
|
||
| extern "C" void* aligned_alloc_hook(size_t alignment, size_t size) { | ||
| void* ret = _orig_aligned_alloc(alignment, size); |
There was a problem hiding this comment.
If my understanding is correct, musl implementation of aligned_alloc calls malloc under the hood; in turn, posix_memalign calls aligned_alloc. Can you verify these are not double counted?
There was a problem hiding this comment.
posix_memaligntriggersposix_memalign_hookas well asaligned_alloc_hook.aligned_alloconly triggersaligned_alloc_hook.
Tested on Alpine 3.21:
profiler.Malloc {
startTime = 17:26:33.908 (2025-02-07)
address = 0x7F3358D43660
size = 28.6 MB
eventThread = "main" (osThreadId = 355)
stackTrace = [
libasyncProfiler.so.Profiler::getNativeTrace() line: 0
libasyncProfiler.so.Profiler::recordSample() line: 0
libasyncProfiler.so.MallocTracer::recordMalloc() line: 0
libasyncProfiler.so.aligned_alloc_hook() line: 0
ld-musl-x86_64.so.1.posix_memalign() line: 0
main.main() line: 0
./lib/ld-musl-x86_64.so.1() line: 0
]
}
profiler.Malloc {
startTime = 17:26:33.908 (2025-02-07)
address = 0x7F3358D43660
size = 28.6 MB
eventThread = "main" (osThreadId = 355)
stackTrace = [
libasyncProfiler.so.Profiler::getNativeTrace() line: 0
libasyncProfiler.so.Profiler::recordSample() line: 0
libasyncProfiler.so.MallocTracer::recordMalloc() line: 0
libasyncProfiler.so.posix_memalign_hook() line: 0
main.main() line: 0
./lib/ld-musl-x86_64.so.1() line: 0
]
}
There was a problem hiding this comment.
OK, thanks. This needs to be fixed then?
test/native/libs/jnimalloc.c
Outdated
| } | ||
|
|
||
| JNIEXPORT jlong JNICALL Java_test_nativemem_Native_alignedAlloc(JNIEnv* env, jclass clazz, jlong alignment, jlong size) { | ||
| return (jlong)aligned_alloc((size_t)alignment, (size_t)size); |
There was a problem hiding this comment.
Interim intptr_t cast needed for 32-bit?
| cc->patchImport(im_realloc, (void*)realloc_hook); | ||
| cc->patchImport(im_free, (void*)free_hook); | ||
| cc->patchImport(im_posix_memalign, (void*)posix_memalign_hook); | ||
| cc->patchImport(im_aligned_alloc, (void*)aligned_alloc_hook); |
There was a problem hiding this comment.
Don't forget to add new hooks to MallocTracer::initialize()
src/mallocTracer.cpp
Outdated
| static thread_local bool in_musl_posix_memalign = false; | ||
| static thread_local bool in_musl_aligned_alloc = false; |
There was a problem hiding this comment.
thread_local may allocate (using malloc!), so it's problematic to use thread_local in malloc hooks.
Why not just skip these hooks entirely on musl? Will malloc_hook catch these functions?
There was a problem hiding this comment.
In musl, the next-gen aligned_alloc implementation no longer relies on malloc, unlike the current-gen version, which uses malloc in certain cases.
As of 2012, posix_memalign has consistently called aligned_alloc instead of using malloc, so hooking posix_memalign in musl is unnecessary.
However, since aligned_alloc no longer necessarily calls malloc, it still requires hooking to ensure proper behavior.
There was a problem hiding this comment.
Wait, new aligned_alloc does call malloc, am I wrong?
There was a problem hiding this comment.
Sorry, yes. I haven't seen this in jfr profiles of aligned_alloc calls. Need to check this again.
src/mallocTracer.cpp
Outdated
| MARK_ASYNC_PROFILER); | ||
|
|
||
| // _CS_GNU_LIBC_VERSION is not defined on musl | ||
| is_musl = confstr(_CS_GNU_LIBC_VERSION, NULL, 0) == 0 && errno != 0; |
There was a problem hiding this comment.
To avoid code duplication, can we move this logic to os_linux.cpp and expose OS::isMusl()?
src/mallocTracer.cpp
Outdated
| // If in_musl_aligned_alloc, do not recordMalloc in malloc_hook. | ||
| // Do not access thread_local variables in non-musl. | ||
| static bool is_musl = false; | ||
| static thread_local bool in_musl_posix_memalign = false; |
There was a problem hiding this comment.
We hit several issues with deadlocks and thread locals.
Our solution was to control the creation and deletion of the thread local variables with get/set_specific.
That implies hooking thread creation to ensure you can create these states. Though I think you already have such hooks.
We also added guards to avoid re-entrance (this avoids double counting mentioned bellow).
I'm not sure if this is preferable to your approach, though feel free to compare.
There was a problem hiding this comment.
Thanks, we are looking into a simpler implementation that does not avoid double-accounting, yet still has correct leak detection.
3 participants
In addition to
malloc,calloc,reallocandfree; nowposix_memalignandaligned_allocare supported for tracking native memory allocations.Related issues
#1121
How has this been tested?
New nativemem test added with
posix_memalignandaligned_allocusage.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.