libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.73k stars 1.8k forks source link

Memory Leak with Newest SDL3 #10273

Closed Dlinuigh closed 3 months ago

Dlinuigh commented 3 months ago

Hey, guys, I use sdl3 for my project, but I find the program keeping swallow memory. I think there might be a leak memory in the code. I have used valgrind and sanitize to analyze the program, and checked my code several times. Clang-tidy can't find any place to complain, so do I. But it still consumes memory at a speed of ~1 MB/s. Here is the sanitize output, as for valgrind, almost the same but more ugly, hence I won't put it out. Anyway, that'll be good if you can tell me something relative. sdl3_memory_leak.log

slouken commented 3 months ago

Graphic::get_tile() keeps allocating surfaces, do you free them anywhere?

Dlinuigh commented 3 months ago

I free all surface when the class call destruction function. And I store all surface in a vectormap.

Dlinuigh commented 3 months ago

Here's class Graphic code.

class Graphic {
  std::map<std::string, SDL_Surface *> textures;
  std::map<char, SDL_Surface *> charmap;
  Json::Value image_doc = {};
  std::map<std::string, glm::ivec2> tileset = {};
  std::string cached_png_name = {};
  int size{};
  Data &data;
  Graphic() : data(Data::getInstance()) { init(); }
  void load_images();
  std::map<std::string, int> images = {};
  SDL_Surface *get_image(const std::string &name);
  void init() { load_images(); }

public:
  Graphic(Graphic const &) = delete;
  void operator=(Graphic const &) = delete;
  SDL_Surface *get_tile(const std::string &name, const std::string &tilename);
  SDL_Surface *get_char(char key, TTF_Font *font, SDL_Color fcolor);
  static Graphic &getInstance() {
    static Graphic instance;
    return instance;
  }
  ~Graphic() {
    for (auto &it : charmap) {
      SDL_DestroySurface(it.second);
    }
    for (auto &it : textures) {
      SDL_DestroySurface(it.second);
    }
  }
};
slouken commented 3 months ago

I don't know, I don't see any leak in SDL itself. If you can strip it down to a simple example just using SDL APIs, that would be helpful.

Dlinuigh commented 3 months ago

Okay, I will do more testing and experiments, but I can't always keep up with the newest version of SDL. I will use only this version.

Dlinuigh commented 3 months ago

I find that the code above caused a memory leak, which is entirely my fault. I mean it relates to SDL_Surface, but the SDL_Renderer-related problem can be simply reproduced by the code below. You may find some unrelated code, as I am just trying to simulate the situation.

#include <SDL3/SDL.h>
#include <SDL3_image/SDL_image.h>
#include <glm/glm.hpp>
#include <map>
#include <string>
class Test {
  std::map<std::string, SDL_Surface *> lut = {};
  SDL_Surface *image = NULL;

public:
  Test() {
    image = IMG_Load("tool.png");
  }
  void load_images() {
    std::map<std::string, glm::ivec2> tileset = {
        {"point", {0, 0}},  {"rect", {1, 0}}, {"eraser", {2, 0}},
        {"bucket", {3, 0}}, {"save", {0, 1}},
    };
    for (auto &it : tileset) {
      SDL_Rect src = {16 * it.second.x, 16 * it.second.y, 16, 16};
      SDL_Surface *tile = SDL_CreateSurface(16, 16, SDL_PIXELFORMAT_ABGR8888);
      SDL_BlitSurface(image, &src, tile, nullptr);
      lut[it.first] = tile;
    }
  }
  ~Test() {
    SDL_DestroySurface(image);
    for (auto &it : lut) {
      SDL_DestroySurface(it.second);
    }
  }
};
int main() {
  SDL_Init(SDL_INIT_VIDEO);
  SDL_Init(SDL_INIT_EVENTS);
  Test test;
  test.load_images();
  SDL_Event event;
  SDL_Window *window =
      SDL_CreateWindow("Test Leak", 60, 60, SDL_WINDOW_BORDERLESS);
  SDL_Renderer *render = SDL_CreateRenderer(window, nullptr);
  do {
    SDL_PollEvent(&event);
    SDL_RenderPresent(render);
  } while (!(event.type == SDL_EVENT_KEY_DOWN));
  SDL_DestroyRenderer(render);
  SDL_DestroyWindow(window);
  SDL_Quit();
  return 0;
}

Here's the sanitized log. leak.log

Here's a simply one.

#include <SDL3/SDL.h>
int main() {
  SDL_Init(SDL_INIT_VIDEO);
  SDL_Init(SDL_INIT_EVENTS);
  SDL_Event event;
  SDL_Window *window =
      SDL_CreateWindow("Test Leak", 60, 60, SDL_WINDOW_BORDERLESS);
  SDL_Renderer *render = SDL_CreateRenderer(window, nullptr);
  do {
    SDL_PollEvent(&event);
    SDL_RenderPresent(render);
  } while (!(event.type == SDL_EVENT_KEY_DOWN));
  SDL_DestroyRenderer(render);
  SDL_DestroyWindow(window);
  SDL_Quit();
  return 0;
}

The log:
leak_simple_test.log

BTW, the program (compressed into a zip): leak.zip

Dlinuigh commented 3 months ago

I got some news, when run the program in a XDG_SESSION_TYPE=x11 environment, for example gnome on xorg, will cause less log:


=================================================================
==46523==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 920 byte(s) in 5 object(s) allocated from:
    #0 0x7a0f806fc34a in calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x7a0f805d2d8a  (<unknown module>)

Direct leak of 520 byte(s) in 13 object(s) allocated from:
    #0 0x7a0f806fca31 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7a0f805e4248  (<unknown module>)

Direct leak of 96 byte(s) in 3 object(s) allocated from:
    #0 0x7a0f806fca31 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7a0f805e0954  (<unknown module>)

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7a0f806fca31 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7a0f805e835f  (<unknown module>)

Indirect leak of 1181 byte(s) in 6 object(s) allocated from:
    #0 0x7a0f806fb6e2 in realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85
    #1 0x7a0f805e0a4f  (<unknown module>)

Indirect leak of 1008 byte(s) in 3 object(s) allocated from:
    #0 0x7a0f806fb6e2 in realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85
    #1 0x7a0f805e18cc  (<unknown module>)

Indirect leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7a0f806fca31 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7a0f805e1288  (<unknown module>)

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7a0f806fca31 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7a0f805e835f  (<unknown module>)

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7a0f806fca31 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7a0f805d887c  (<unknown module>)

SUMMARY: AddressSanitizer: 3837 byte(s) leaked in 34 allocation(s).
Dlinuigh commented 3 months ago

So, Wayland is really a naughty boy.

Dlinuigh commented 3 months ago

What most exciting is the leak message is fake, even run program in a wayland environment won't consume more memory. I guess I can live with this.

Dlinuigh commented 3 months ago

Summary of this problem: SDL_Surface* leak is a real leak, SDL_Renderer Leak is a fake leak and only appear in a wayland environment but still no effect.