SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Valgrind reports memory leak in SinkInfoCallback

Open dareg opened this issue 8 months ago • 4 comments

With SDL 3.2.10, on Debian 12.10, compiled with gcc 14.1.0

Valgrind's memcheck reports a memory leak on the code below. I am not familiar with the internal SDL code, but it seems strange that the return value of SDL_AddAudioDevice at the line 785 of SDL_Pulseaudio.c is ignored. Doesn't it mean that the handle given in argument is never freed?

#include <SDL3/SDL.h>

int
main(int argc, char* argv[])
{
  if (!SDL_InitSubSystem(SDL_INIT_AUDIO)) {
    return 1;
  }

  SDL_QuitSubSystem(SDL_INIT_AUDIO);
  SDL_Quit();
  return 0;
}

When running it with valgrind:

$ valgrind --leak-check=full ./a.out 
==321717== Memcheck, a memory error detector
==321717== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==321717== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==321717== Command: ./a.out
==321717== 
==321717== 
==321717== HEAP SUMMARY:
==321717==     in use at exit: 86,529 bytes in 243 blocks
==321717==   total heap usage: 1,151 allocs, 908 frees, 329,000 bytes allocated
==321717== 
==321717== 129 (72 direct, 57 indirect) bytes in 1 blocks are definitely lost in loss record 70 of 99
==321717==    at 0x48416C4: malloc (vg_replace_malloc.c:380)
==321717==    by 0x494578: real_realloc (SDL_malloc.c:6329)
==321717==    by 0x494832: SDL_realloc_REAL (SDL_malloc.c:6489)
==321717==    by 0x49A600: SDL_SetTLS_REAL (SDL_thread.c:93)
==321717==    by 0x49AB9F: SDL_GetErrBuf (SDL_thread.c:303)
==321717==    by 0x5A7DFF: SDL_SetErrorV_REAL (SDL_error.c:43)
==321717==    by 0x5A7DCB: SDL_SetError_REAL (SDL_error.c:33)
==321717==    by 0x5B1B10: SDL_FindPhysicalAudioDeviceByCallback (SDL_audio.c:1487)
==321717==    by 0x5B1B55: SDL_FindPhysicalAudioDeviceByHandle (SDL_audio.c:1500)
==321717==    by 0x5AF494: SDL_AddAudioDevice (SDL_audio.c:684)
==321717==    by 0x69A675: AddPulseAudioDevice (SDL_pulseaudio.c:785)
==321717==    by 0x69A6C4: SinkInfoCallback (SDL_pulseaudio.c:794)
==321717== 
==321717== LEAK SUMMARY:
==321717==    definitely lost: 72 bytes in 1 blocks
==321717==    indirectly lost: 57 bytes in 2 blocks
==321717==      possibly lost: 0 bytes in 0 blocks
==321717==    still reachable: 86,400 bytes in 240 blocks
==321717==         suppressed: 0 bytes in 0 blocks
==321717== Reachable blocks (those to which a pointer was found) are not shown.
==321717== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==321717== 
==321717== For lists of detected and suppressed errors, rerun with: -s
==321717== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

dareg avatar Apr 03 '25 20:04 dareg

Hmm, it's SDL_SetError() in the hotplug thread. What's the correct way to clean this up? It's storing it in a TLS var.

icculus avatar Apr 03 '25 20:04 icculus

If the hotplug thread isn't created by SDL, it could call SDL_CleanupTLS() before returning.

slouken avatar Apr 03 '25 20:04 slouken

This should be fixed by 3b91017682146b42e95a5bfcefbc9013615cb3b8, but let me know if it still pops up somewhere after that change, @dareg, and we'll reopen the issue!

icculus avatar Apr 06 '25 03:04 icculus

Fix had unintended consequences, reopening to revisit this issue.

icculus avatar May 11 '25 14:05 icculus