We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

This is similar to bug 1003

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.

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.

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

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.

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

Reopening for a quick fix.

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!

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.

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

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

Thanks!

Created attachment 754 [details]
segfault

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!

(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?

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?

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.

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?

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);
                }