search

eclipse-mosquitto / mosquitto

Eclipse Mosquitto - An open source MQTT broker
https://mosquitto.org
Other
9.06k stars 2.39k forks source link

Memory leak with per_listener_settings set to true #2788

Open manthanrtilva opened 1 year ago

manthanrtilva commented 1 year ago

OS info

# cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

mosquitto info

# git clone https://github.com/eclipse/mosquitto.git
# cd mosquitto
# cmake -DCMAKE_BUILD_TYPE=Debug .
# make -j
# src/mosquitto -h
mosquitto version 2.0.15

mosquitto is an MQTT v5.0/v3.1.1/v3.1 broker.

Usage: mosquitto [-c config_file] [-d] [-h] [-p port]
# git show
commit 4e6fbae45ce424d2204c8b5d51b37dc5a08013bc (HEAD -> master, origin/release/2.0,
origin/master, origin/HEAD)
Author: Roger A. Light <roger@atchoo.org>
Date:   Sun Jan 15 22:18:31 2023 +0000

    Delete old workflow runs.

How to reproduce memory leak.

  1. Open default mosquitto.conf
  2. Add below line at end (Note. mosquitto_payload_modification.so is just as example for any other plugin there is same behavior) 
    per_listener_settings true
listener 8883
plugin plugins/payload-modification/mosquitto_payload_modification.so
  3. Run with valgrind and stop (with CTL+C) after few seconds 
    
# valgrind --leak-check=full --show-reachable=yes --error-limit=no src/mosquitto -c mosquitto.conf
==1357== Memcheck, a memory error detector
==1357== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1357== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==1357== Command: src/mosquitto -c mosquitto.conf
==1357==
1681278260: mosquitto version 2.0.15 starting
1681278260: Config loaded from mosquitto.conf.
1681278260: Loading plugin: plugins/payload-modification/mosquitto_payload_modification.so
1681278260: Opening ipv4 listen socket on port 8883.
1681278260: Opening ipv6 listen socket on port 8883.
1681278260: mosquitto version 2.0.15 running
^C1681278262: mosquitto version 2.0.15 terminating
==1357==
==1357== HEAP SUMMARY:
==1357==     in use at exit: 327 bytes in 2 blocks
==1357==   total heap usage: 4,377 allocs, 4,375 frees, 204,389 bytes allocated
==1357==
==1357== 63 bytes in 1 blocks are indirectly lost in loss record 1 of 2
==1357==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==1357==    by 0x4DB8F3A: strdup (strdup.c:42)
==1357==    by 0x425FFB: mosquitto__strdup (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x417320: conf__parse_string (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x411D43: config__read_file_core (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x416BA8: config__read_file (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x410D01: config__read (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x40FA99: config__parse_args (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x426E13: main (in /tmp/mosquitto/src/mosquitto)
==1357==
==1357== 327 (264 direct, 63 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 2
==1357==    at 0x48386AF: malloc (vg_replace_malloc.c:306)
==1357==    by 0x483ADE7: realloc (vg_replace_malloc.c:834)
==1357==    by 0x425F5F: mosquitto__realloc (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x411C70: config__read_file_core (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x416BA8: config__read_file (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x410D01: config__read (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x40FA99: config__parse_args (in /tmp/mosquitto/src/mosquitto)
==1357==    by 0x426E13: main (in /tmp/mosquitto/src/mosquitto)
==1357==
==1357== LEAK SUMMARY:
==1357==    definitely lost: 264 bytes in 1 blocks
==1357==    indirectly lost: 63 bytes in 1 blocks
==1357==      possibly lost: 0 bytes in 0 blocks
==1357==    still reachable: 0 bytes in 0 blocks
==1357==         suppressed: 0 bytes in 0 blocks
==1357==
==1357== For lists of detected and suppressed errors, rerun with: -s
==1357== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)


<!--

STOP!  If you are reporting a security issue, which includes *anything* that
       may cause the broker/client to crash, please do NOT report it here but
       follow the steps at https://www.eclipse.org/security/

If your issue is not a bug or a feature request, please use the mailing list at
https://dev.eclipse.org/mailman/listinfo/mosquitto-dev to ask your question.
There are many more people available to help there than on this issue tracker.

If you are reporting a bug PLEASE include the version of Mosquitto you are
using and what platform (Windows, Ubuntu/Fedora/... Linux, FreeBSD, ...) you
are running on.

Please also note that some systems have old versions of Mosquitto available in
their package repositories. We would be very grateful if you would check to
see whether the bug is still present in a newer version before submitting your
issue.

-->
manthanrtilva commented 1 year ago

Here is PR to resolve this memory leak. https://github.com/eclipse/mosquitto/pull/2787

ralight commented 1 year ago

Thank you for the report, and for the PR. I am in two minds about this. On one hand, I prefer to be completely valgrind clean. On the other hand, this is a tiny leak that happens when the broker is exiting anyway, so it doesn't really matter. The related code is completely refactored in the develop branch and so this issue will not be present in 2.1.

Merging the PR may well produce conflicts when merging back into the develop branch, which I'd really prefer to avoid.

On balance, I'd like to leave this as it is. Saving a single 330 byte leak per plugin and that happens only on exit, doesn't feel like a good reason to make life more difficult. What do you think?

manthanrtilva commented 1 year ago

@ralight Sound good to leave it, if it will be getting resolved in next version.