naemon / naemon-livestatus

Naemon - Livestatus Eventbroker Module
GNU General Public License v2.0
26 stars 30 forks source link

Hotfix/open inet sock memory leak #69

Closed chifac08 closed 4 years ago

chifac08 commented 4 years ago

The new implemented TCP Interface has an Memory Leak when it exits with an error. I am aware that this normally should not cause any issues but it's not nice and valgrind also complains about it. This makes it hard to find other memory leaks.

The following configuration causes the Memory Leak: livestatus.cfg # Naemon config broker_module=/usr/local/lib/naemon-livestatus/livestatus.so inet_addr=0.0.0.0:0 event_broker_options=-1

To force an error i set the port to 0!

I made a memchek with valgrind. Here is the output:

==2127== Memcheck, a memory error detector
==2127== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2127== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==2127== Command: naemon --allow-root /usr/local/etc/naemon/naemon.cfg
==2127== Parent PID: 2047
==2127== 
==2127== 
==2127== HEAP SUMMARY:
==2127==     in use at exit: 168,840 bytes in 704 blocks
==2127==   total heap usage: 5,959 allocs, 5,255 frees, 572,619 bytes allocated
==2127== 
==2127== 10 bytes in 1 blocks are definitely lost in loss record 31 of 362
==2127==    at 0x483A7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2127==    by 0x4AD153E: strdup (strdup.c:42)
==2127==    by 0x512D584: open_inet_socket (module.c:280)
==2127==    by 0x512E831: nebmodule_init (module.c:757)
==2127==    by 0x488A9FF: neb_load_module (nebmods.c:221)
==2127==    by 0x488AB87: neb_load_all_modules (nebmods.c:153)
==2127==    by 0x10C530: main (naemon.c:573)
==2127== 
==2127== LEAK SUMMARY:
==2127==    definitely lost: 10 bytes in 1 blocks
==2127==    indirectly lost: 0 bytes in 0 blocks
==2127==      possibly lost: 0 bytes in 0 blocks
==2127==    still reachable: 168,830 bytes in 703 blocks
==2127==         suppressed: 0 bytes in 0 blocks
==2127== Reachable blocks (those to which a pointer was found) are not shown.
==2127== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==2127== 
==2127== For lists of detected and suppressed errors, rerun with: -s
==2127== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

To fix these and any other leaks that is caused by strdup in this method, i freed the memory on error exit.

A new valgrind run shows that it fixed the leak because the save pointer gets freed on error exit.

==10075== LEAK SUMMARY:
==10075==    definitely lost: 0 bytes in 0 blocks
==10075==    indirectly lost: 0 bytes in 0 blocks
==10075==      possibly lost: 0 bytes in 0 blocks
==10075==    still reachable: 168,834 bytes in 703 blocks
==10075==         suppressed: 0 bytes in 0 blocks
==10075== Reachable blocks (those to which a pointer was found) are not shown.
==10075== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==10075== 
==10075== For lists of detected and suppressed errors, rerun with: -s
==10075== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

I would be delighted if you could review my pull request and give me some feedback!

thx!

jacobbaungard commented 4 years ago

Squashed the commits and merged (https://github.com/naemon/naemon-livestatus/commit/2e2b86678b69945f94a43f9a4dfc6ef4d6e39c24).