Skip to content

JACK clients do not free all allocated memory when closed #1001

Description

@JWZ1996

Describe the bug

JACK clients do not free the whole memory after closing.

I already identified the problematic code place and have a solution. See it below.

Environment

  • JACK Version: Reproduced at 1.9.20 and 1.9.22 (current master)
  • Operating System: Ubuntu 22.04
  • Installation: Standard apt installation

Steps To Reproduce

  1. Write and compile any JACK client
// jack_client.c
#include <jack/jack.h>

jack_client_t *client;

int main(int argc, char *argv[]) {
    const char *client_name = "passthrough";
    jack_options_t options = JackNullOption;
    jack_status_t status;

    client = jack_client_open(client_name, options, &status);
    jack_client_close(client);
    
    return 0;
}
$ gcc -o jack_client jack_client.c `pkg-config --cflags --libs jack`
  1. Run it with valgrind:
$ valgrind --leak-check=full --show-leak-kinds=all ./jack_client

Expected vs. actual behavior

Actual behavior:
Output will be: (I have JACK lib with debug symbols)

==30216== Memcheck, a memory error detector
==30216== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==30216== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==30216== Command: ./jack_client
==30216== 
==30216== 
==30216== HEAP SUMMARY:
==30216==     in use at exit: 96 bytes in 2 blocks
==30216==   total heap usage: 62 allocs, 60 frees, 200,493 bytes allocated
==30216== 
==30216== 48 bytes in 1 blocks are still reachable in loss record 1 of 2
==30216==    at 0x4849013: operator new(unsigned long) (vg_replace_malloc.c:422)
==30216==    by 0x48862AD: __static_initialization_and_destruction_0 (JackGlobals.cpp:36)
==30216==    by 0x48862AD: _GLOBAL__sub_I_JackGlobals.cpp (JackGlobals.cpp:81)
==30216==    by 0x400647D: call_init.part.0 (dl-init.c:70)
==30216==    by 0x4006567: call_init (dl-init.c:33)
==30216==    by 0x4006567: _dl_init (dl-init.c:117)
==30216==    by 0x40202C9: ??? (in /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
==30216== 
==30216== 48 bytes in 1 blocks are still reachable in loss record 2 of 2
==30216==    at 0x4849013: operator new(unsigned long) (vg_replace_malloc.c:422)
==30216==    by 0x48862CB: __static_initialization_and_destruction_0 (JackGlobals.cpp:37)
==30216==    by 0x48862CB: _GLOBAL__sub_I_JackGlobals.cpp (JackGlobals.cpp:81)
==30216==    by 0x400647D: call_init.part.0 (dl-init.c:70)
==30216==    by 0x4006567: call_init (dl-init.c:33)
==30216==    by 0x4006567: _dl_init (dl-init.c:117)
==30216==    by 0x40202C9: ??? (in /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
==30216== 
==30216== LEAK SUMMARY:
==30216==    definitely lost: 0 bytes in 0 blocks
==30216==    indirectly lost: 0 bytes in 0 blocks
==30216==      possibly lost: 0 bytes in 0 blocks
==30216==    still reachable: 96 bytes in 2 blocks
==30216==         suppressed: 0 bytes in 0 blocks
==30216== 
==30216== For lists of detected and suppressed errors, rerun with: -s
==30216== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Expected behavior is valgrind showing no errors. Valgrind also should return 0, which I want for automatic integration test script for my app. I also spent a lot of time debugging resource releasing, before I realized it's in the jack lib, I wouldn't want others to waste their time on this either.

Solution

Problematic code lines are JackGlobals.cpp:37 and JackGlobals.cpp:36.

Here are 2 JackMutex objects, allocated with operator new:

JackMutex* JackGlobals::fOpenMutex = new JackMutex();

JackMutex* JackGlobals::fOpenMutex = new JackMutex();
JackMutex* JackGlobals::fSynchroMutex = new JackMutex();

When you search for both symbols, you will find out that there is no call of delete for those objects!:
https://github.com/search?q=repo%3Ajackaudio/jack2%20fOpenMutex&type=code
https://github.com/search?q=repo%3Ajackaudio/jack2%20fSynchroMutex&type=code

So that memory is never released.

Solution is to replace it with a smart pointer, for C++ '11:

// JackGlobals.h
static std::unique_ptr<JackMutex> fOpenMutex;
static std::unique_ptr<JackMutex> fSynchroMutex;

// JackGlobals.cpp
std::unique_ptr<JackMutex> JackGlobals::fOpenMutex  { new JackMutex() };
std::unique_ptr<JackMutex> JackGlobals::fSynchroMutex { new JackMutex() };

std::unique_ptr<JackMutex> and JackMutex* have exactly the same API, so no more changes are required.

I already prepared and tested a branch with such fix locally, please let me know if can I open a PR! :)

Thanks,
Jan

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions