Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory leaks #137

Closed
1 task done
4eUeP opened this issue Jul 28, 2022 · 1 comment
Closed
1 task done

Memory leaks #137

4eUeP opened this issue Jul 28, 2022 · 1 comment

Comments

@4eUeP
Copy link

4eUeP commented Jul 28, 2022 •
edited
Loading

Hi, I'm trying to include this library in grpc_bench. However, there were some critical memory leaks that failed the bench.

Reproduce

Example server code: LesnyRumcajs/grpc_bench#243

Start server:

# ./setup_scenario.sh string_10B && cd haskell_grpc_haskell_bench && make && cabal build
valgrind --leak-check=yes $(cabal exec -- which haskell-grpc-haskell-bench)

A simple request:

ghz -c 1 --connections=1 -n 1 --insecure --proto proto/helloworld/helloworld.proto -d '{"request": {"name": "x"}}' --call helloworld.Greeter.SayHello 127.0.0.1:50051

Valgrind output:

==6972== Memcheck, a memory error detector
==6972== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6972== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==6972== Command: /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench
==6972==
==6972== Warning: set address range perms: large range [0x4200000000, 0x14200100000) (noaccess)
^C==6972==
==6972== Process terminating with default action of signal 2 (SIGINT)
==6972==    at 0x4D2E3DB: kill (syscall-template.S:78)
==6972==    by 0x211DB28: exitBySignal (RtsStartup.c:627)
==6972==    by 0x211DDD1: shutdownHaskellAndSignal (RtsStartup.c:592)
==6972==    by 0x1FC12D9: ??? (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==
==6972== HEAP SUMMARY:
==6972==     in use at exit: 70,456 bytes in 17 blocks
==6972==   total heap usage: 1,104 allocs, 1,087 frees, 522,254 bytes allocated
==6972==
==6972== 0 bytes in 1 blocks are definitely lost in loss record 1 of 14
==6972==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6972==    by 0x15605A3: op_send_initial_metadata (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==    by 0x1538B7B: grpczmhaskellzmcorezm0zi3zi0zm835310fa19e8773ca754a75c240f6e73c79cc57e1c1ee407f6993ad725919bb0_NetworkziGRPCziLowLevelziOp_zdwsetOpArray_info (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==
==6972== 0 bytes in 1 blocks are definitely lost in loss record 2 of 14
==6972==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6972==    by 0x156076D: op_send_status_server (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==    by 0x150E183: grpczmhaskellzmcorezm0zi3zi0zm835310fa19e8773ca754a75c240f6e73c79cc57e1c1ee407f6993ad725919bb0_NetworkziGRPCziUnsafeziOp_zdwopSendStatusServer_info (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==
==6972== 16 bytes in 1 blocks are definitely lost in loss record 7 of 14
==6972==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6972==    by 0x155FDA1: grpc_completion_queue_pluck_ (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==    by 0x152C698: grpczmhaskellzmcorezm0zi3zi0zm835310fa19e8773ca754a75c240f6e73c79cc57e1c1ee407f6993ad725919bb0_NetworkziGRPCziUnsafe_zdwgrpcCompletionQueuePluck_info (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==    by 0x8000000000000000: ???
==6972==
==6972== 320 bytes in 1 blocks are definitely lost in loss record 12 of 14
==6972==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6972==    by 0x4C5C391: gpr_malloc (in /usr/local/lib/libgpr.so.14.0.0)
==6972==    by 0x4B2AE6F: grpc_raw_compressed_byte_buffer_create (in /usr/local/lib/libgrpc.so.14.0.0)
==6972==    by 0x156062D: op_send_message (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==    by 0x1538AE7: grpczmhaskellzmcorezm0zi3zi0zm835310fa19e8773ca754a75c240f6e73c79cc57e1c1ee407f6993ad725919bb0_NetworkziGRPCziLowLevelziOp_zdwsetOpArray_info (in /srv/grpc_bench/haskell_grpc_haskell_bench/dist-newstyle/build/x86_64-linux/ghc-8.10.7/grpc-haskell-bench-0.1.0.0/x/haskell-grpc-haskell-bench/build/haskell-grpc-haskell-bench/haskell-grpc-haskell-bench)
==6972==
==6972== LEAK SUMMARY:
==6972==    definitely lost: 336 bytes in 4 blocks
==6972==    indirectly lost: 0 bytes in 0 blocks
==6972==      possibly lost: 0 bytes in 0 blocks
==6972==    still reachable: 70,120 bytes in 13 blocks
==6972==         suppressed: 0 bytes in 0 blocks
==6972== Reachable blocks (those to which a pointer was found) are not shown.
==6972== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==6972==
==6972== For lists of detected and suppressed errors, rerun with: -s
==6972== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)

Leaks

  • memory allocated by grpc_byte_buffer_copy fromop_send_message seems redundant
  • memory allocated by malloc from op_send_initial_metadata
  • memory allocated by malloc from op_send_status_server
  • memory allocated by malloc from grpc_completion_queue_pluck_
@ixmatus
Copy link
Contributor

ixmatus commented Aug 8, 2022

Hi, thank you for creating this ticket!

mrBliss added a commit to mrBliss/gRPC-haskell that referenced this issue Sep 28, 2022
@mrBliss
Fix memory leak in op_send_initial_metadata
7e956c0 
There is no corresponding `free` for this `malloc` call in
`op_send_initial_metadata`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
mrBliss added a commit to mrBliss/gRPC-haskell that referenced this issue Sep 28, 2022
@mrBliss
Fix memory leak in op_send_message
f8d4dd8 
There is no corresponding `grpc_byte_buffer_destroy` for this
`grpc_byte_buffer_copy` call in `op_send_message`, as `op`s are not recursively
`free`d.

Fix it by referring directly to the byte buffer instead of copying it, as the
byte buffer will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
mrBliss added a commit to mrBliss/gRPC-haskell that referenced this issue Oct 7, 2022
@mrBliss
Fix memory leak in op_send_status_server
0c43dbb 
There is no corresponding `free` for this `malloc` call in
`op_send_status_server`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
@mrBliss mrBliss mentioned this issue Oct 7, 2022
Closed
fumieval pushed a commit to fumieval/gRPC-haskell that referenced this issue Jun 22, 2023
@mrBliss @fumieval
Fix memory leak in op_send_initial_metadata
d01a8f1 
There is no corresponding `free` for this `malloc` call in
`op_send_initial_metadata`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
fumieval pushed a commit to fumieval/gRPC-haskell that referenced this issue Jun 22, 2023
@mrBliss @fumieval
Fix memory leak in op_send_message
b2596a9 
There is no corresponding `grpc_byte_buffer_destroy` for this
`grpc_byte_buffer_copy` call in `op_send_message`, as `op`s are not recursively
`free`d.

Fix it by referring directly to the byte buffer instead of copying it, as the
byte buffer will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
fumieval pushed a commit to fumieval/gRPC-haskell that referenced this issue Jun 22, 2023
@mrBliss @fumieval
Fix memory leak in op_send_status_server
10060c5 
There is no corresponding `free` for this `malloc` call in
`op_send_status_server`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
fumieval pushed a commit to fumieval/gRPC-haskell that referenced this issue Jul 18, 2023
@mrBliss @fumieval
Fix memory leak in op_send_initial_metadata
d622277 
There is no corresponding `free` for this `malloc` call in
`op_send_initial_metadata`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
fumieval pushed a commit to fumieval/gRPC-haskell that referenced this issue Jul 18, 2023
@mrBliss @fumieval
Fix memory leak in op_send_message
50f9a7d 
There is no corresponding `grpc_byte_buffer_destroy` for this
`grpc_byte_buffer_copy` call in `op_send_message`, as `op`s are not recursively
`free`d.

Fix it by referring directly to the byte buffer instead of copying it, as the
byte buffer will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
fumieval pushed a commit to fumieval/gRPC-haskell that referenced this issue Jul 18, 2023
@mrBliss @fumieval
Fix memory leak in op_send_status_server
9352490 
There is no corresponding `free` for this `malloc` call in
`op_send_status_server`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: awakesecurity#137
ixmatus pushed a commit that referenced this issue Jul 18, 2023
@fumieval @mrBliss
Rebase fix memory leak (#153)
d20c20d 
* Fix memory leak in op_send_initial_metadata

There is no corresponding `free` for this `malloc` call in
`op_send_initial_metadata`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: #137

* Fix memory leak in op_send_message

There is no corresponding `grpc_byte_buffer_destroy` for this
`grpc_byte_buffer_copy` call in `op_send_message`, as `op`s are not recursively
`free`d.

Fix it by referring directly to the byte buffer instead of copying it, as the
byte buffer will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: #137

* Work around memory leak in `createOpContext`

According to valgrind, a `grpc_slice` created by `grpc_slice_malloc_` (called
via `C.grpcSliceMalloc` in the `OpRecvStatusOnClient` case of `createOpContext`)
is leaked:

```
72 bytes in 1 blocks are definitely lost in loss record 8 of 11
   at 0x484079B: malloc (in /nix/store/73if83n79h4ml8rcb473ym47zmb4vi5x-valgrind-3.19.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x4BD2ACD: gpr_malloc (in /nix/store/kfbhv9kid1dzblf16w9g1i10x48xkyrr-grpc-1.34.1/lib/libgpr.so.14.0.0)
   by 0x4AC3864: grpc_core::UnmanagedMemorySlice::HeapInit(unsigned long) (in /nix/store/kfbhv9kid1dzblf16w9g1i10x48xkyrr-grpc-1.34.1/lib/libgrpc.so.14.0.0)
   by 0x4AC3AC0: grpc_slice_malloc (in /nix/store/kfbhv9kid1dzblf16w9g1i10x48xkyrr-grpc-1.34.1/lib/libgrpc.so.14.0.0)
   by 0x41DCD1: grpc_slice_malloc_ (in ..)
   by 0x48D758: grpczmleakzm0zi0zi1zminplace_NetworkziGRPCziLowLevelziOp_createOpContext1_info (in ..)
```

However, we actually try to free this slice using `free_slice` in
`freeOpContext`, so this is unexpected.

Note that `grpc_slice` stores its payload on the heap and maintains its own
reference count, unless the payload is small enough to be stored inline. Perhaps
there is still an unknown reference to it somewhere, which keeps the
`grpc_slice` alive. I haven't found out why.

I did discover that decreasing `defaultStatusStringLen` and thus the size of the
slice made the memory leak disappear. I believe because a smaller size means
that no extra heap allocation is necessary and the payload can be stored inline.

As the comment on `defaultStatusStringLen` says that "gRPC actually ignores this
length and reallocates a longer string if necessary", decreasing its size should
be safe.

* Fix memory leak in op_send_status_server

There is no corresponding `free` for this `malloc` call in
`op_send_status_server`, as `op`s are not recursively `free`d.

Fix it by referring directly to the metadata instead of copying it, as the
metadata will remain alive for the lifetime of the `Op`, see
`withOpArrayAndCtxts`.

Source: #137

---------

Co-authored-by: Thomas Winant <[email protected]>
@4eUeP 4eUeP closed this as completed May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ixmatus @4eUeP