JACK clients do not free all allocated memory when closed

[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`

2. 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:

jack2/common/JackGlobals.cpp

Line 36 in 43e1173

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

[jpka-]

I can confirm this 96 bytes leak. (gcc 15.1.1, jack2 1.9.22-1, Archlinux)
echo 'int main() { return 0; }' | gcc -ljack -o /tmp/test -xc - && valgrind --leak-check=full --show-leak-kinds=all /tmp/test