Memory leak in mpdas, or in libmpdclient?

[quite]

I'm a user of mpdas, and observing the process growing slowly over time (it also seems to happen when I'm not playing anything in mpd, but I'm not certain of that).

I've run mpdas through valgrind for quiet some time, with the following result. Have to say I'm no expert in interpreting this. But when reading the relevant code in mpdas (also pasted below), it seems like the right things are done and freed...

==16708== 
==16708== HEAP SUMMARY:
==16708==     in use at exit: 84,345,057 bytes in 1,304,903 blocks
==16708==   total heap usage: 2,683,628 allocs, 1,378,725 frees, 1,628,698,875 bytes allocated
==16708== 
==16708== 13,360 bytes in 863 blocks are possibly lost in loss record 20 of 26
==16708==    at 0x4C28C10: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16708==    by 0x5BD6E09: strdup (in /usr/lib/libc-2.22.so)
==16708==    by 0x4E41EA6: ??? (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E423ED: mpd_song_feed (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E426AA: mpd_recv_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E3EA85: mpd_run_current_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4065BA: CMPD::Update() (in /usr/local/bin/mpdas)
==16708==    by 0x4039F0: main (in /usr/local/bin/mpdas)
==16708== 
==16708== 18,041 bytes in 203 blocks are possibly lost in loss record 21 of 26
==16708==    at 0x4C28C10: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16708==    by 0x5BD6E09: strdup (in /usr/lib/libc-2.22.so)
==16708==    by 0x4E41D38: ??? (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E42682: mpd_recv_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E3EA85: mpd_run_current_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4065BA: CMPD::Update() (in /usr/local/bin/mpdas)
==16708==    by 0x4039F0: main (in /usr/local/bin/mpdas)
==16708== 
==16708== 47,680 bytes in 149 blocks are possibly lost in loss record 22 of 26
==16708==    at 0x4C28C10: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16708==    by 0x4E41D24: ??? (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E42682: mpd_recv_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E3EA85: mpd_run_current_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4065BA: CMPD::Update() (in /usr/local/bin/mpdas)
==16708==    by 0x4039F0: main (in /usr/local/bin/mpdas)
==16708== 
==16708== 84,183,316 (54,879,360 direct, 29,303,956 indirect) bytes in 171,498 blocks are definitely lost in loss record 26 of 26
==16708==    at 0x4C28C10: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16708==    by 0x4E41D24: ??? (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E42682: mpd_recv_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4E3EA85: mpd_run_current_song (in /usr/lib/libmpdclient.so.2.0.10)
==16708==    by 0x4065BA: CMPD::Update() (in /usr/local/bin/mpdas)
==16708==    by 0x4039F0: main (in /usr/local/bin/mpdas)
==16708== 
==16708== LEAK SUMMARY:
==16708==    definitely lost: 54,879,360 bytes in 171,498 blocks
==16708==    indirectly lost: 29,303,956 bytes in 1,132,013 blocks
==16708==      possibly lost: 79,081 bytes in 1,215 blocks
==16708==    still reachable: 82,660 bytes in 177 blocks
==16708==         suppressed: 0 bytes in 0 blocks
==16708== Reachable blocks (those to which a pointer was found) are not shown.
==16708== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==16708== 
==16708== For counts of detected and suppressed errors, rerun with: -v
==16708== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)

    mpd_status *status = mpd_run_status(_conn);
    mpd_stats *stats = mpd_run_stats(_conn);

    if(status && stats) {
        int newsongid = mpd_status_get_song_id(status);
        int newsongpos = mpd_status_get_elapsed_time(status);
        int curplaytime = mpd_stats_get_play_time(stats);
        mpd_song *song = mpd_run_current_song(_conn);

        // new song
        if(newsongid != _songid) {
            _songid = newsongid;
            _songpos = newsongpos;
            _start = curplaytime;

            if(song) {
                GotNewSong(song);
                mpd_song_free(song);
            }
        }

        // song playing
        if(newsongpos != _songpos) {
            _songpos = newsongpos;
            CheckSubmit(curplaytime);
        }

        // check for client-to-client messages
        if(mpd_send_read_messages(_conn)) {
            mpd_message *msg;
            while((msg = mpd_recv_message(_conn)) != NULL) {
                const char *text = mpd_message_get_text(msg);
                if(_gotsong && text && !strncmp(text, "love", 4))
                    AudioScrobbler->LoveTrack(_song);
                mpd_message_free(msg);
            }
            mpd_response_finish(_conn);
        }

        mpd_status_free(status);
        mpd_stats_free(stats);
    }
    else { // we have most likely lost our connection
        eprintf("Could not query MPD server: %s", mpd_connection_get_error_message(_conn));
        _connected = false;
    }

[hrkfdn]

Thanks for the report. It seems that the song return value is only free'd when a new song is playing. This could be the source of the memory leak. I'll have to do some further research, though. Could you try moving "mpd_song *song = mpd_run_current_song(_conn);" inside the if-statements block for handling new songs and see if the issue persists?

[quite]

Aha! I'll do that, and get back when enough playing/non-playing time has passed...

[hrkfdn]

Awesome, thanks for the assistance. I'd do it myself but can't right now..

[hrkfdn]

Merged, thanks a lot for the patch + testing! :)

[timss]

Thanks, had noticed it was going out of control myself.

Might want to do a new release if you feel it's ready, @hrkfdn

[hrkfdn]

Done!