Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[raudio] Failing to load a MP3-File causes a double free #3889

Closed
FishingHacks opened this issue Mar 29, 2024 · 11 comments
Closed

[raudio] Failing to load a MP3-File causes a double free #3889

FishingHacks opened this issue Mar 29, 2024 · 11 comments
Labels
help needed - please! I need help with this issue

Comments

@FishingHacks
Copy link
Contributor

FishingHacks commented Mar 29, 2024

Issue description

When using LoadMusicStream() on a file that is not valid mp3, raylib crashes with free(): double free detected in tcache 2.

I tried digging a bit deeper and upon further (tho not very detailed) inspection, i found out that drmp3_init_file closes the file if it could not successfully load (dr_mp3.h:3536). pclose is later called again by drmp3_uninit (raudio.c:1448:54). drmp3_uninit seems to be causing the issue as removing it fixes it. I also noticed that drmp3_uninit calls drmp3__free_from_callbacks, but that shouldnt be called anyway because drmp3_init_internal calls it when it fails (dr_mp3.h:2854:9).

Environment

Platform: Desktop

Code Example

#include "raylib/src/raylib.h"
#include "stdio.h"

int main(void) {
    InitAudioDevice();
    if (!IsAudioDeviceReady()) {
        printf("couldn't initialize the audio device :notlikethis:");
        return -1;
    }

    Music stream = LoadMusicStream("music.mp3");

    PlayMusicStream(stream);

    while (IsMusicStreamPlaying(stream)) {
        UpdateMusicStream(stream);
    }

    UnloadMusicStream(stream);

    CloseAudioDevice();

    return 0;
}
@FishingHacks
Copy link
Contributor Author

Checking other formats, the .wav format also has the same double free error message, so i suspect its the same issue with drwav_uninit. The QOA Format segfaults, i didn't investigate yet why tho

@RobLoach
Copy link
Contributor

RobLoach commented Apr 3, 2024

Able to reproduce a similar issue with the audio_music_stream example if you delete resources/country.mp3.

WARNING: FILEIO: [resources/country.mp3] Music file could not be opened
INFO: TIMER: Target time per frame: 33.333 milliseconds
audio_music_stream: raylib/src/external/dr_mp3.h:3775: drmp3_seek_to_start_of_stream: Assertion `pMP3 != ((void *)0)' failed.
Aborted (core dumped)

@veins1
Copy link
Contributor

veins1 commented Apr 13, 2024

Able to reproduce a similar issue with the audio_music_stream example if you delete resources/country.mp3.

WARNING: FILEIO: [resources/country.mp3] Music file could not be opened
INFO: TIMER: Target time per frame: 33.333 milliseconds
audio_music_stream: raylib/src/external/dr_mp3.h:3775: drmp3_seek_to_start_of_stream: Assertion `pMP3 != ((void *)0)' failed.
Aborted (core dumped)

This is fixed by #3917 but it was a separate issue. Original issue still holds.

@raysan5
Copy link
Owner

raysan5 commented Apr 21, 2024

@RobLoach @veins1 @FishingHacks I'm reviewing this issue but I can't reproduce it. Using raylib latest version from GitHub master branch. Please, could you provide some sample code/example to review it?

@veins1
Copy link
Contributor

veins1 commented Apr 21, 2024

@raysan5 To reproduce you can create an empty file and rename it to file.mp3 . Then modify audio_music_stream.c so that it loads this file.

@raysan5 raysan5 added the help needed - please! I need help with this issue label May 5, 2024
veins1 added a commit to veins1/raylib that referenced this issue May 7, 2024
Fix for raysan5#3889
Fixes for QOA crashes.
Memory leak FIX on unsuccessful .wav loading.
raysan5 pushed a commit that referenced this issue May 7, 2024
* Fixes for loading Music

Fix for #3889
Fixes for QOA crashes.
Memory leak FIX on unsuccessful .wav loading.

* Added comments
@FishingHacks
Copy link
Contributor Author

FishingHacks commented May 14, 2024

Okay, sorry for the late reply, have been busy lately. The Pull Request #3966 seems to have fixed most issues, ogg, mp3, qoa, flac, xm and mod files seem to work. wav files are still broken. Taking a quick look at the pull request, it calls drwav_uninit if the initialization failed (see raudio.c:1351), but doesn't do the same thing for mp3 (see raudio.c:1399). I believe that this is the issue.

My code (main.c):

#include "raylib.h"
#include "stdio.h"

int main(int argc, char **argv) {
    if (argc != 2) {
        printf("usage: main [file]");
        return -2;
    }

    InitAudioDevice();
    if (!IsAudioDeviceReady()) {
        printf("couldn't initialize the audio device");
        return -1;
    }

    Music stream = LoadMusicStream(argv[1]);

    PlayMusicStream(stream);

    while (IsMusicStreamPlaying(stream)) {
        UpdateMusicStream(stream);
    }

    UnloadMusicStream(stream);

    CloseAudioDevice();

    return 0;
}

I ran the following commands (assuming we start out in an empty directory with the above source in main.c):

$ sudo apt install build-essential git libasound2-dev libx11-dev libxrandr-dev libxi-dev libgl1-mesa-dev libglu1-mesa-dev libxcursor-dev libxinerama-dev libwayland-dev libxkbcommon-dev
$ git clone https://github.com/raysan5/raylib.git
$ cd raylib/src
$ make PLATFORM=PLATFORM_DESKTOP
$ cd ../../
$ cp ./raylib/src/raylib.h .
$ cp ./raylib/src/libraylib.a .
$ gcc -o main main.c -I. -L. -lraylib -lGL -lm -lpthread -ldl -lrt -lX11
$ touch test.flac test.mod test.mp3 test.ogg test.qoa test.wav test.xm
$ ./main test.flac
$ ./main test.mod
$ ./main test.mp3
$ ./main test.ogg
$ ./main test.qoa
$ ./main test.wav
$ ./main test.xm

@veins1
Copy link
Contributor

veins1 commented May 15, 2024

Couldn't reproduce the .wav crash, but going through the code in a debugger, drwav_uninit doesn't do anything useful in our case. I think it's safe to remove it, can you confirm that removing it fixes the crash?

@raysan5
Copy link
Owner

raysan5 commented May 15, 2024

@FishingHacks @veins1 Thank you very much for reviewing this issue! Feel free to send a PR if you confirm removing drwav_uninit fixes the crash. Thanks! :)

@FishingHacks
Copy link
Contributor Author

That did indeed fix the issue i was having, i created a pull request (#3986) to fix this

PS: Thanks for your incredible work on raylib

@raysan5 raysan5 closed this as completed May 16, 2024
@raysan5
Copy link
Owner

raysan5 commented May 16, 2024

@FishingHacks thank you very much for reviewing this issue! 😄

@FishingHacks
Copy link
Contributor Author

no problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help needed - please! I need help with this issue
Projects
None yet
Development

No branches or pull requests

4 participants