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.

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

Allocate buffer memory for uv_write and release in the callback function #12688

Merged
merged 2 commits into from Apr 19, 2022

Conversation

stelfrag
Copy link
Collaborator

Fixes #12685

Summary

Allocate buffers needed for uv_write and let the callback function release the memory

Test Plan
  • Alert notifications should work

@stelfrag stelfrag marked this pull request as ready for review April 15, 2022 18:55
@stelfrag stelfrag requested a review from Ferroin as a code owner April 15, 2022 18:55
@stelfrag stelfrag requested review from erdem2000, odynik, MrZammler and thiagoftsm and removed request for Ferroin April 15, 2022 18:55
Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the PR, and I received all expected notifications. I also used valgrind 3.17, and I did not receive memory leak notification for this code. LGTM!

@thiagoftsm
Copy link
Contributor

@stelfrag the next warning does not have relationship with your PR, but I observed it during my tests:

==8655== 848 bytes in 1 blocks are still reachable in loss record 2,659 of 2,764
==8655==    at 0x48467C5: malloc (vg_replace_malloc.c:380)
==8655==    by 0x431C79: mallocz (libnetdata.c:176)
==8655==    by 0x574D8B: spawn_client (spawn_client.c:176)
==8655==    by 0x5627052: start_thread (in /lib64/libc-2.35.so)
==8655==    by 0x56B3BEF: clone (in /lib64/libc-2.35.so)

I also observed that we are not cleaning properly the spawn server:

==8655== 416 bytes in 1 blocks are possibly lost in loss record 2,616 of 2,764
==8655==    at 0x484B5FF: calloc (vg_replace_malloc.c:1117)
==8655==    by 0x4013F95: _dl_allocate_tls (in /lib64/ld-2.35.so)
==8655==    by 0x5627D24: pthread_create@@GLIBC_2.34 (in /lib64/libc-2.35.so)
==8655==    by 0x4E1EECC: uv_thread_create_ex (in /usr/lib64/libuv.so.1.0.0)
==8655==    by 0x4E1EF7B: uv_thread_create (in /usr/lib64/libuv.so.1.0.0)
==8655==    by 0x5738C6: spawn_init (spawn.c:243)
==8655==    by 0x41BF8C: main (main.c:1218)

I got the messages running:

valgrind --leak-check=full --show-leak-kinds=all --log-file=errors.log  /usr/sbin/netdata -D

best regards!

@stelfrag stelfrag merged commit bbff41d into netdata:master Apr 19, 2022
@stelfrag stelfrag deleted the 0_spawn_process branch April 19, 2022 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Spawn server / client uses uv_write with buffers in the stack
3 participants