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

Memory leak when recreating Player on loop in Python #335

Open
Architector4 opened this issue Jun 14, 2024 · 2 comments
Open

Memory leak when recreating Player on loop in Python #335

Architector4 opened this issue Jun 14, 2024 · 2 comments
Assignees
Labels

Comments

@Architector4
Copy link

I have a Python script that I have running once a second, which uses Playerctl to fetch data about currently running players.

The only way I found to do so is to create a new Player for each player name from list_players(), because PlayerManager doesn't get information about newly appearing or closing players unless I recreate it in loop too. But this occupies more and more memory each time, even if invoke Python's garbage collector after each loop.

Same issue seems to occur even if I preserve

My script and the problem could be boiled down to this:

import gi
gi.require_version('Playerctl', '2.0')
from gi.repository import Playerctl

while True:
    for name in Playerctl.list_players():
        player = Playerctl.Player.new_from_name(name)
        #print(player.get_title())

Running this slowly drains more and more system memory. I may be missing a better way about this though, but I'm unsure.

Architector4 added a commit to Architector4/dotfiles-n-stuff that referenced this issue Jul 10, 2024
A better compromise for me due to a memory leak
that I otherwise experience:
altdesktop/playerctl#335
@ungive
Copy link

ungive commented Nov 30, 2024

I'm facing the same issue, which is especially noticeable when a media player reports the cover image in the MPRIS artUrl field in the form of a data: URI, e.g. data:image/png;base64,... which can be multiple megabytes of data.

To reproduce:

  • Install the Elisa media player: https://apps.kde.org/elisa (I'm using version 23.08.5)
  • Open a song with a cover image of at least 1 MB (Mine was 2 MB in size)
  • Play the song for a few seconds at least (after that it should be ok for it to be paused)
  • Run a program using code similar to the code below and observe spiking memory usage

This is a minimal reproducible example in C++ using the C playerctl library, version 2.4.1:

#include "playerctl.h"
extern "C" {
#include "playerctl-common.h"
}
#define defer(name__, block__) \
    std::shared_ptr<void> name__(nullptr, std::bind([&] { \
        try { block__; } catch (...) { assert(false); } \
    }))
static void observe(std::function<bool()> run)
{
    GError* error = nullptr;
    PlayerctlPlayerManager* manager = playerctl_player_manager_new(&error);
    if (error != nullptr) throw std::runtime_error(error->message);
    defer(free_manager, g_object_unref(manager));
    while (run()) {
        // Get all available players
        GList *available_players = nullptr;
        g_object_get(manager, "player-names", &available_players, nullptr);
        if (available_players == nullptr) throw std::runtime_error("no players");
        available_players = g_list_copy(available_players);
        defer(free_available_players, g_list_free(available_players));
        // Iterate each player
        GList *l = nullptr;
        for (l = available_players; l != nullptr; l = l->next) {
            // Player
            PlayerctlPlayerName *player_name = reinterpret_cast<PlayerctlPlayerName *>(l->data);
            PlayerctlPlayer *player = playerctl_player_new_from_name(player_name, &error);
            if (error != nullptr) throw std::runtime_error(error->message);
            if (player == nullptr) throw std::runtime_error("player is null");
            defer(free_player, g_object_unref(player));
            std::string player_id = player_name->instance;
            // Metadata
            GVariant *raw_metadata = nullptr;
            g_object_get(player, "metadata", &raw_metadata, nullptr);
            if (raw_metadata == nullptr) throw std::runtime_error("no metadata");
            defer(free_raw_metadata, g_variant_unref(raw_metadata));
        }
        std::this_thread::sleep_for(std::chrono::milliseconds{ 10 });
    }
}

While writing this I've been closely following how playerctl-cli.c is written and read notes about when to free in the GLib documentation, to make sure I'm doing everything right, yet it still has memory leaks.

Running this with Elisa opened and a song with a 2 MB cover that is reported via MPRIS with a data: URL makes this program take up 1 GB of memory in under 10 seconds! Reducing the looping frequency to once every second (1000ms) still causes it to leak about 100MB within a minute, rendering the program unusable with a media player like Elisa and possibly other media players that report cover images in this way.

Running this with Valgrind gives the following output (truncated to the relevant parts):

$ libtool exec valgrind --tool=memcheck --leak-check=full --log-file=valgrind.txt ./a.out

==1209344== Memcheck, a memory error detector
==1209344== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==1209344== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
...
==1209344== HEAP SUMMARY:
==1209344==     in use at exit: 1,048,006,721 bytes in 29,331 blocks
==1209344==   total heap usage: 1,271,776 allocs, 1,242,445 frees, 1,126,251,392 bytes allocated
...
==1209344== 2,951,086 (64 direct, 2,951,022 indirect) bytes in 1 blocks are definitely lost in loss record 1,579 of 1,582
==1209344==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1209344==    by 0x6398AF9: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x63D85C2: g_variant_builder_end (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x6C629E7: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C62AFA: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6222D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6290D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C629AC: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C2B080: g_dbus_message_new_from_blob (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C37542: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6BC67AA: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6BCA702: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344== 
==1209344== 374,414,161 bytes in 127 blocks are possibly lost in loss record 1,580 of 1,582
==1209344==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1209344==    by 0x6398AF9: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x63AE562: g_memdup2 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x63675A1: g_bytes_new (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x63DA40C: g_variant_new_string (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x6C62638: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C62AFA: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6222D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6290D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C62AFA: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6222D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6290D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344== 
==1209344== 667,318,816 (22,592 direct, 667,296,224 indirect) bytes in 353 blocks are definitely lost in loss record 1,581 of 1,582
==1209344==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1209344==    by 0x6398AF9: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x63D85C2: g_variant_builder_end (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.8000.0)
==1209344==    by 0x6C629E7: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C62AFA: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6222D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C6290D: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C629AC: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C2B080: g_dbus_message_new_from_blob (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6C37542: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6BC67AA: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344==    by 0x6BC67E4: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.8000.0)
==1209344== 
==1209344== LEAK SUMMARY:
==1209344==    definitely lost: 24,438 bytes in 388 blocks
==1209344==    indirectly lost: 670,283,656 bytes in 26,044 blocks
==1209344==      possibly lost: 374,416,318 bytes in 141 blocks
==1209344==    still reachable: 3,260,293 bytes in 2,565 blocks
==1209344==         suppressed: 0 bytes in 0 blocks

Commenting everything below // Metadata out seemingly removes the memory leak:

            // // Metadata
            // GVariant *raw_metadata = nullptr;
            // g_object_get(player, "metadata", &raw_metadata, nullptr);
            // if (raw_metadata == nullptr) throw std::runtime_error("no metadata");
            // defer(free_raw_metadata, g_variant_unref(raw_metadata));

glib version: 2.80.0-6ubuntu3.2

EDIT: Compiled glib from the master branch to exclude it being the issue, still same memory leak: https://github.com/GNOME/glib/tree/c50836535a10e142b27cd8b7d7c97c8d114d3598

@ungive
Copy link

ungive commented Dec 1, 2024

Modifying the loop to run the metadata retrieval only once and linking with glib compiled from source (https://github.com/GNOME/glib/tree/c50836535a10e142b27cd8b7d7c97c8d114d3598) yields this valgrind output:

==36721== 2,950,614 (64 direct, 2,950,550 indirect) bytes in 1 blocks are definitely lost in loss record 1,568 of 1,568
==36721==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==36721==    by 0x639E261: g_malloc (gmem.c:100)
==36721==    by 0x63E5591: g_variant_alloc (gvariant-core.c:582)
==36721==    by 0x63E5591: g_variant_new_from_children (gvariant-core.c:801)
==36721==    by 0x63E170A: g_variant_builder_end (gvariant.c:3826)
==36721==    by 0x6C51ACE: parse_value_from_blob.isra.0 (gdbusmessage.c:2125)
==36721==    by 0x6C51CB8: parse_value_from_blob.isra.0 (gdbusmessage.c:2177)
==36721==    by 0x6C51460: parse_value_from_blob.isra.0 (gdbusmessage.c:2064)
==36721==    by 0x6C51BBB: parse_value_from_blob.isra.0 (gdbusmessage.c:2005)
==36721==    by 0x6C51A93: parse_value_from_blob.isra.0 (gdbusmessage.c:2109)
==36721==    by 0x6C5463C: g_dbus_message_new_from_blob (gdbusmessage.c:2497)
==36721==    by 0x6C5F68F: _g_dbus_worker_do_read_cb (gdbusprivate.c:752)
==36721==    by 0x6BEEB52: g_task_return_now (gtask.c:1363)

This happens to be exactly the size of the cover image (encoded as a data URL ~3MB).

Decrementing the reference count of the metadata GVariant appears to fix the leak in my case:

            GVariant *raw_metadata = nullptr;
            g_object_get(player, "metadata", &raw_metadata, nullptr);
            if (raw_metadata == nullptr) throw std::runtime_error("no metadata");
            defer(unref_raw_metadata, {
                g_variant_unref(raw_metadata);
                g_variant_unref(raw_metadata);
            });

It seems there's a g_variant_unref call missing for the metadata field specifically (not other fields) somewhere in the playerctl code. This workaround seems to work well for now though.

I've looked at the OP's code again and playerctl_player_get_title calls playerctl_player_print_metadata_prop which calls playerctl_player_get_metadata, so this most likely has the same underlying cause.

Bug reports with similar stack traces, might be useful:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants