Skip to content

Commit

Permalink
tools/memleak: Fix data race on --combined-only
Browse files Browse the repository at this point in the history
combined_allocs map can be modified by multiple threads and it causes data
race easily. To prevent this issue, use atomic operation, like
libbpf-tools/memleak.bpf.c does. This atomic implementation cannot resolve
all data race cases but reduce some.

Followings are the way to generate data race issue and the result after
applying this patch.

File test.cpp

  #include <iostream>
  #include <thread>
  #include <unistd.h>

  void alloc() {
    for (int i = 0; i < 100000; ++i) {
      int* a = (int*)malloc(4);
    }
  }

  int main() {
    sleep(100);
    std::thread t1 {&alloc};
    std::thread t2 {&alloc};
    t1.join();
    t2.join();
    sleep(50);
    return 0;
  }

Build the test file

  $ g++ test.cpp

Run test program and memleak.py without --combined-only

  $ ./a.out &
  [1] 65168
  $ sudo python3 memleak.py -p 65168
  Attaching to pid 65168, Ctrl+C to quit.
  [11:52:57] Top 10 stacks with outstanding allocations:
  ...snip...
  [11:54:09] Top 10 stacks with outstanding allocations:
        576 bytes in 2 allocations from stack
                0x00007ff3f6bba7da      _dl_allocate_tls+0x3a [ld-linux-x86-64.so.2]
        799992 bytes in 199998 allocations from stack
                0x000055bc2bff12c8      alloc()+0x1f [a.out]
                0x000055bc2bff1bd3      void std::__invoke_impl<void, void (*)()>(std::__invoke_other, void (*&&)())+0x21 [a.out]
                0x000055bc2bff1b91      std::__invoke_result<void (*)()>::type std::__invoke<void (*)()>(void (*&&)())+0x24 [a.out]
                0x000055bc2bff1b32      void std::thread::_Invoker<std::tuple<void (*)()> >::_M_invoke<0ul>(std::_Index_tuple<0ul>)+0x2c [a.out]
                0x000055bc2bff1b02      std::thread::_Invoker<std::tuple<void (*)()> >::operator()()+0x1c [a.out]
                0x000055bc2bff1ae2      std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)()> > >::_M_run()+0x20 [a.out]
                0x00007ff3f68dc253      [unknown] [libstdc++.so.6.0.30]
        16785408 bytes in 2 allocations from stack
                0x00007ff3f6495707      pthread_create+0xac7 [libc.so.6]
        134217728 bytes in 1 allocations from stack
                0x00007ff3f64a1c82      alloc_new_heap+0xc2 [libc.so.6]

Run test program and memleak.py with --combined-only

  $ ./a.out &
  [1] 64183
  [11:40:56] Top 10 stacks with outstanding allocations:
  ...snip...
  [11:42:06] Top 10 stacks with outstanding allocations:
        0 bytes in 0 allocations from stack
                operator new(unsigned long)+0x1c [libstdc++.so.6.0.30]
                main+0x44 [a.out]
                __libc_start_call_main+0x80 [libc.so.6]
        0 bytes in 0 allocations from stack
                operator new(unsigned long)+0x1c [libstdc++.so.6.0.30]
                main+0x62 [a.out]
                __libc_start_call_main+0x80 [libc.so.6]
        576 bytes in 2 allocations from stack
                _dl_allocate_tls+0x3a [ld-linux-x86-64.so.2]
        664816 bytes in 166204 allocations from stack
                alloc()+0x1f [a.out]
                void std::__invoke_impl<void, void (*)()>(std::__invoke_other, void (*&&)())+0x21 [a.out]
                std::__invoke_result<void (*)()>::type std::__invoke<void (*)()>(void (*&&)())+0x24 [a.out]
                void std::thread::_Invoker<std::tuple<void (*)()> >::_M_invoke<0ul>(std::_Index_tuple<0ul>)+0x2c [a.out]
                std::thread::_Invoker<std::tuple<void (*)()> >::operator()()+0x1c [a.out]
                std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)()> > >::_M_run()+0x20 [a.out]
                [unknown] [libstdc++.so.6.0.30]
        16785408 bytes in 2 allocations from stack
                pthread_create+0xac7 [libc.so.6]
        134217728 bytes in 1 allocations from stack
                alloc_new_heap+0xc2 [libc.so.6]

The count value difference between 199998 and 166204 shows the data race issue.

After Applying this patch, run memleak.py with --combined-only

  $ ./a.out &
  [1] 63828
  [11:36:31] Top 10 stacks with outstanding allocations:
  ...snip...
  [11:37:59] Top 10 stacks with outstanding allocations:
        576 bytes in 2 allocations from stack
                _dl_allocate_tls+0x3a [ld-linux-x86-64.so.2]
        799992 bytes in 199998 allocations from stack
                alloc()+0x1f [a.out]
                void std::__invoke_impl<void, void (*)()>(std::__invoke_other, void (*&&)())+0x21 [a.out]
                std::__invoke_result<void (*)()>::type std::__invoke<void (*)()>(void (*&&)())+0x24 [a.out]
                void std::thread::_Invoker<std::tuple<void (*)()> >::_M_invoke<0ul>(std::_Index_tuple<0ul>)+0x2c [a.out]
                std::thread::_Invoker<std::tuple<void (*)()> >::operator()()+0x1c [a.out]
                std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)()> > >::_M_run()+0x20 [a.out]
                [unknown] [libstdc++.so.6.0.30]
        16785408 bytes in 2 allocations from stack
                pthread_create+0xac7 [libc.so.6]
        134217728 bytes in 1 allocations from stack
                alloc_new_heap+0xc2 [libc.so.6]
  • Loading branch information
Bojun-Seo authored and yonghong-song committed Jan 13, 2024
1 parent 5e92141 commit 3d21000
Showing 1 changed file with 18 additions and 21 deletions.
39 changes: 18 additions & 21 deletions tools/memleak.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,35 +169,32 @@ def run_command_get_pid(command):
static inline void update_statistics_add(u64 stack_id, u64 sz) {
struct combined_alloc_info_t *existing_cinfo;
struct combined_alloc_info_t cinfo = {0};
struct combined_alloc_info_t cinfo = {0, 0};
existing_cinfo = combined_allocs.lookup(&stack_id);
if (existing_cinfo != 0)
cinfo = *existing_cinfo;
cinfo.total_size += sz;
cinfo.number_of_allocs += 1;
combined_allocs.update(&stack_id, &cinfo);
if (!existing_cinfo) {
combined_allocs.update(&stack_id, &cinfo);
existing_cinfo = combined_allocs.lookup(&stack_id);
if (!existing_cinfo)
return;
}
__sync_fetch_and_add(&existing_cinfo->total_size, sz);
__sync_fetch_and_add(&existing_cinfo->number_of_allocs, 1);
}
static inline void update_statistics_del(u64 stack_id, u64 sz) {
struct combined_alloc_info_t *existing_cinfo;
struct combined_alloc_info_t cinfo = {0};
existing_cinfo = combined_allocs.lookup(&stack_id);
if (existing_cinfo != 0)
cinfo = *existing_cinfo;
if (sz >= cinfo.total_size)
cinfo.total_size = 0;
else
cinfo.total_size -= sz;
if (cinfo.number_of_allocs > 0)
cinfo.number_of_allocs -= 1;
combined_allocs.update(&stack_id, &cinfo);
if (!existing_cinfo)
return;
if (existing_cinfo->number_of_allocs > 1) {
__sync_fetch_and_sub(&existing_cinfo->total_size, sz);
__sync_fetch_and_sub(&existing_cinfo->number_of_allocs, 1);
} else {
combined_allocs.delete(&stack_id);
}
}
static inline int gen_alloc_enter(struct pt_regs *ctx, size_t size) {
Expand Down

0 comments on commit 3d21000

Please sign in to comment.