This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Demo of handle leak bug, and workaround (bad workaround) (sdl_mixer-mod-bug.tar.gz, application/x-gzip, 2010-07-13 10:17:55 +0000, 8364 bytes)
• Correct file open/close sequence (test.c, application/octet-stream, 2011-12-31 15:41:40 +0000, 711 bytes)
• segfault (sdl_mixer-mod-bug-0.2.tar.gz, application/x-gzip, 2012-01-01 07:14:52 +0000, 4619 bytes)

Reported in version: 1.2.11
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2010-07-13 10:17:55 +0000, Peter Kosyh wrote:

Created attachment 515
Demo of handle leak bug, and workaround (bad workaround)

It's like, that there is no SDL_RWclose call in Mix_FreeMusic logic for MUS_MOD muisic. It's leads to massive handle leaking while loading/unloading tracker music, which is heavily used in my project. I made an attach. It'is a small test, compiled with- and without -D_SDL_MOD_BUG. The output of test must looks like this:

---

Compiling and run without workaround.
fd = 136
Compiling and run with workaround hack.
fd = 8

The main idea of the test code:

for (i = 0; i < 128; i++) {
rw = SDL_RWFromFile("test.mod", "rb");
mus = Mix_LoadMUS_RW(rw);
#ifdef _SDL_MOD_BUG
if (Mix_GetMusicType(mus) == MUS_MOD)
SDL_RWclose(rw);
#endif
Mix_HaltMusic();
Mix_FreeMusic(mus);
}
fd = open("test.mod", O_RDONLY);
close(fd);
fprintf(stderr, "fd = %d\n", fd);

On 2011-02-17 13:31:55 +0000, Sam Lantinga wrote:

This is similar to bug 1003

On 2011-04-09 11:15:21 +0000, Matthew Orlando wrote:

I'm not sure this is a bug.

The RWFrom___ functions give you a rwop object that has a distinct lifetime from the music object you get from LoadMUS_RW. I don't think it should be FreeMusic's responsibility to be aware of the original source of the data.

There may be times when you want to keep the rwop in memory. But if not, then you can just free it with SDL_RWclose immediately after calling LoadMUS_RW (just like your workaround). If you left it up to FreeMusic, you would have both the rwop's data and the music object's data in memory for the entire combined lifetime.

On 2011-04-09 23:23:27 +0000, Peter Kosyh wrote:

Ok, forget about my workaround (btw, it is works correctly ONLY with tracker music, in other modules it leads to sigsegv) BUT handle leaks happens while using plain:

Mix_LoadMUS

Mix_FreeMusic

pair, and it happens ONLY with libmikmod. I write demo to show this, leaks are there.

(In reply to comment # 2)

I'm not sure this is a bug.

The RWFrom___ functions give you a rwop object that has a distinct lifetime
from the music object you get from LoadMUS_RW. I don't think it should be
FreeMusic's responsibility to be aware of the original source of the data.

There may be times when you want to keep the rwop in memory. But if not, then
you can just free it with SDL_RWclose immediately after calling LoadMUS_RW
(just like your workaround). If you left it up to FreeMusic, you would have
both the rwop's data and the music object's data in memory for the entire
combined lifetime.

On 2011-04-09 23:25:59 +0000, Matthew Orlando wrote:

Yeah, that seems to be the same problem as the bug Sam mentioned, as well as bug 1168.

On 2011-12-31 11:44:50 +0000, Sam Lantinga wrote:

This isn't technically a bug. The intention for SDL_RWops that are passed to Mix_LoadMUS_RW() is that you free them yourself after you call Mix_FreeMusic().

This is awkward, and other API functions that take SDL_RWops also take a 'freesrc' parameter to indicate the behavior you want. This will be improved in a future release where we unify the way music and other sounds are loaded and handled.

On 2011-12-31 11:49:33 +0000, Sam Lantinga wrote:

Whoops, I see that it's a problem just with LoadMUS()

Reopening for a quick fix.

On 2011-12-31 15:39:14 +0000, Sam Lantinga wrote:

This isn't technically a bug, as I mentioned above. The API assumes that you manage the life cycle of the rwops structure and close it after Mix_FreeMusic().

There was a memory leak when using the file interface, which has been fixed:
http://hg.libsdl.org/SDL_mixer/rev/565549e046b0

Thanks!

On 2011-12-31 15:41:40 +0000, Sam Lantinga wrote:

Created attachment 753
Correct file open/close sequence

I added an attachment to show you the expected way to handle rwops in combination with the current SDL_mixer music API.

As I said, in the future the API will be improved so you won't have to keep track of the rwops structure.

On 2012-01-01 03:48:18 +0000, Peter Kosyh wrote:

(In reply to comment # 8)

Created attachment 753 [details]
Correct file open/close sequence

I added an attachment to show you the expected way to handle rwops in
combination with the current SDL_mixer music API.

As I said, in the future the API will be improved so you won't have to keep
track of the rwops structure.

Please, just put test.ogg file and try to run test with test.ogg (not mod): ->rw = SDL_RWFromFile("test.ogg", "rb");

You will see segfault. The problem is here:

1. some types of music will free rwops in Mix_FreeMusic(mus);
2. some types (i got it ONLY for MUS_MOD, will no free rwops.

So, if (Mix_GetMusicType(mus) == MUS_MOD) line is still requred here :(.

On 2012-01-01 06:10:52 +0000, Sam Lantinga wrote:

Can you attach a new test.c and a new test.ogg that shows the problem? I'm not seeing it here.

Thanks!

On 2012-01-01 07:14:52 +0000, Peter Kosyh wrote:

Created attachment 754
segfault

On 2012-01-01 14:06:53 +0000, Sam Lantinga wrote:

Are you using the latest source from Mercurial?

Here are my results:
samlantinga-pc:sdl_mixer-mod-bug 2 slouken$ sh test.sh
fd = 5

If you are using the latest source, can you debug it and find out why it's crashing?

Thanks!

On 2012-01-01 21:19:06 +0000, Peter Kosyh wrote:

(In reply to comment # 12)

Are you using the latest source from Mercurial?

Here are my results:
samlantinga-pc:sdl_mixer-mod-bug 2 slouken$ sh test.sh
fd = 5

If you are using the latest source, can you debug it and find out why it's
crashing?

Thanks!

Oh, i am not using hg version. I am using 1.2.11 in my distro. And compiled windows version in windows port of my program. While running test i got double free in all cases.

[peter@royal sdl_mixer-mod-bug]$ ./test.sh
*** glibc detected *** ./test: double free or corruption (!prev):
0x0000000001d07d60 ***
======= Backtrace: =========
/lib/libc.so.6(+0x73396)[0x7f74cfefc396]
/lib/libc.so.6(cfree+0x6c)[0x7f74cff0026c]
/lib/libc.so.6(fclose+0x155)[0x7f74cfeec8d5]
/usr/lib/libSDL-1.2.so.0(+0x103e9)[0x7f74d068b3e9]
...

In fact, when i founded this first time, i looked in SDL-mixer code and found that in other music types (at least ogg) library will do SDL_Rwclose after free music, but no with SDL_MOD. Sorry, it was a long time ago, and i do not remember exactly lines.

So, currently, i do not see, how library MUST work.

1. if i will do SDL_RWclose(rw) everywhere, my code will segfault in current versions in all distros.

2. if i will use my hack, my program will not run on newer version? B.t.w it is work well with SDL 1.3 too.

I can look in SDL-mixer code again, when i will have time, but please, tell me, what behaviour is supposed to be right?

We must always call >SDL_RWclose(rw) after Mix_FreeMusic(mus) if it was loaded by Mix_LoadMUS_RW(rw)?

Currently, i am using my hack in Windows/WinCE/Android/S60 (androind uses SDL 1.3) versions. In other versions i got handle leak with MODS (but it is better, then crash). If i will place SDL_RWclose(rw) without condition, my programm will stop work with ogg/mp3.... Please, give me advice what can i do here?

On 2012-01-01 21:34:19 +0000, Peter Kosyh wrote:

Yes. I have no crash with hg version of SDL-mixer.

So, i am really need an advice.

What i must write in my code, to make my project work with current version and future one?

On 2012-01-01 21:39:31 +0000, Sam Lantinga wrote:

This statement should be correct:
We must always call >SDL_RWclose(rw) after Mix_FreeMusic(mus) if it was loaded
by Mix_LoadMUS_RW(rw)?

This isn't true for SMPEG, but that's a bug in SMPEG which should be addressed separately.

On 2012-01-01 21:43:09 +0000, Peter Kosyh wrote:

Ok, i have build SDL-mixer from tarball and bug is here:

*** glibc detected *** ./test: double free or corruption (!prev): 0x0000000001274f80 ***
======= Backtrace: =========
/lib/libc.so.6(+0x73396)[0x7f1a56236396]
/lib/libc.so.6(cfree+0x6c)[0x7f1a5623a26c]
/lib/libc.so.6(fclose+0x155)[0x7f1a562268d5]
/usr/lib/libSDL-1.2.so.0(+0x103e9)[0x7f1a567773e9]
/usr/lib/libvorbisfile.so.3(ov_clear+0xa0)[0x7f1a4d4494e0]
./libSDL_mixer-1.2.so.0.10.1(OGG_delete+0x28)[0x7f1a56a1b718]
./libSDL_mixer-1.2.so.0.10.1(Mix_FreeMusic+0xa9)[0x7f1a56a0e499]
./test[0x400a63]

So, it's like, that there is no bug in HG version and it is exist in tarball.

So, can you give me advice, how to write ifdefs to make code work in both versions?

On 2012-01-03 19:33:48 +0000, Sam Lantinga wrote:

Okay, this is what you want:

            int freeMusic = 1;
            rw = SDL_RWFromFile("test.ogg", "rb");
            mus = Mix_LoadMUS_RW(rw);
            Mix_HaltMusic();
            Mix_FreeMusic(mus);

#if SDL_VERSIONNUM(SDL_MIXER_MAJOR_VERSION, SDL_MIXER_MINOR_VERSION, SDL_MIXER_PATCHLEVEL) < SDL_VERSIONNUM(1, 2, 12)
if (Mix_GetMusicType(mus) == MUS_OGG) {
freeMusic = 0;
}
#endif
if (Mix_GetMusicType(mus) == MUS_MP3) {
freeMusic = 0;
}
if (freeMusic) {
SDL_RWclose(rw);
}