Skip to content

Commit

Permalink
Set our recursion guard in all hooks
Browse files Browse the repository at this point in the history
This prevents reentrancy if one hooked function calls into another.
For instance, the jemalloc implementation of `malloc` calls into `mmap`
while holding a lock, and our `mmap` hook calls `malloc`, so we must
prevent our `mmap` hook from running beneath our `malloc` hook.

Signed-off-by: Matt Wozniski <godlygeek@gmail.com>
  • Loading branch information
godlygeek authored and pablogsal committed Jul 29, 2023
1 parent a519a95 commit 9ac0989
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 10 deletions.
1 change: 1 addition & 0 deletions news/433.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug leading to a deadlock when Memray is used to profile an application that uses the jemalloc implementation of ``malloc``.
52 changes: 42 additions & 10 deletions src/memray/_memray/hooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,14 @@ malloc(size_t size) noexcept
{
assert(hooks::malloc);

void* ptr = hooks::malloc(size);
tracking_api::Tracker::trackAllocation(ptr, size, hooks::Allocator::MALLOC);
void* ptr;
{
tracking_api::RecursionGuard guard;
ptr = hooks::malloc(size);
}
if (ptr) {
tracking_api::Tracker::trackAllocation(ptr, size, hooks::Allocator::MALLOC);
}
return ptr;
}

Expand All @@ -182,7 +188,10 @@ free(void* ptr) noexcept
tracking_api::Tracker::trackDeallocation(ptr, 0, hooks::Allocator::FREE);
}

hooks::free(ptr);
{
tracking_api::RecursionGuard guard;
hooks::free(ptr);
}
}

void*
Expand Down Expand Up @@ -224,8 +233,14 @@ void*
mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) noexcept
{
assert(hooks::mmap);
void* ptr = hooks::mmap(addr, length, prot, flags, fd, offset);
tracking_api::Tracker::trackAllocation(ptr, length, hooks::Allocator::MMAP);
void* ptr;
{
tracking_api::RecursionGuard guard;
ptr = hooks::mmap(addr, length, prot, flags, fd, offset);
}
if (ptr != MAP_FAILED) {
tracking_api::Tracker::trackAllocation(ptr, length, hooks::Allocator::MMAP);
}
return ptr;
}

Expand All @@ -234,8 +249,14 @@ void*
mmap64(void* addr, size_t length, int prot, int flags, int fd, off64_t offset) noexcept
{
assert(hooks::mmap64);
void* ptr = hooks::mmap64(addr, length, prot, flags, fd, offset);
tracking_api::Tracker::trackAllocation(ptr, length, hooks::Allocator::MMAP);
void* ptr;
{
tracking_api::RecursionGuard guard;
ptr = hooks::mmap64(addr, length, prot, flags, fd, offset);
}
if (ptr != MAP_FAILED) {
tracking_api::Tracker::trackAllocation(ptr, length, hooks::Allocator::MMAP);
}
return ptr;
}
#endif
Expand All @@ -245,7 +266,10 @@ munmap(void* addr, size_t length) noexcept
{
assert(hooks::munmap);
tracking_api::Tracker::trackDeallocation(addr, length, hooks::Allocator::MUNMAP);
return hooks::munmap(addr, length);
{
tracking_api::RecursionGuard guard;
return hooks::munmap(addr, length);
}
}

void*
Expand Down Expand Up @@ -303,7 +327,11 @@ dlclose(void* handle) noexcept
{
assert(hooks::dlclose);

int ret = hooks::dlclose(handle);
int ret;
{
tracking_api::RecursionGuard guard;
ret = hooks::dlclose(handle);
}
tracking_api::NativeTrace::flushCache();
if (!ret) tracking_api::Tracker::invalidate_module_cache();
return ret;
Expand Down Expand Up @@ -349,7 +377,11 @@ pvalloc(size_t size) noexcept
{
assert(hooks::pvalloc);

void* ret = hooks::pvalloc(size);
void* ret;
{
tracking_api::RecursionGuard guard;
ret = hooks::pvalloc(size);
}
if (ret) {
tracking_api::Tracker::trackAllocation(ret, size, hooks::Allocator::PVALLOC);
}
Expand Down

0 comments on commit 9ac0989

Please sign in to comment.