OpenTESArena crashes when using a release build of WildMIDI

[Allofich]

Link to the original issue afritz1/OpenTESArena#69.

OpenTESArena uses WildMidi. If run with a Debug or RelWithDebugInfo build of WildMIDI, no crash happens, but if run with a Release build, a crash always happens. Only tested on Windows 7, 64-bit.

Steps to reproduce crash:
First you need OpenTESArena running with MIDI.
It requires a copy of The Elder Scrolls: Arena (the free, floppy-disk version released by Bethesda Softworks) to be installed. It's easy to find. Next,

1. Download the latest OpenTESArena release (https://github.com/afritz1/OpenTESArena/releases).
2. Get the eawpats MIDI sound patch library:
   https://github.com/afritz1/OpenTESArena/releases/download/opentesarena-0.1.0/eawpats.zip
   or
   https://github.com/afritz1/OpenTESArena/releases/download/opentesarena-0.1.0/eawpats.tar.gz
   and put the extracted contents into the OpenTESArena data folder.

Make sure you set the path to your installation of Arena in the OpenTESArena options file. The easiest thing to do if you just want to quickly get the game up and running is to directly modify the default in the options-default.txt in the OpenTESArena options folder.

Try the game, it will run fine.
Now replace the supplied wildmidi_dynamic.dll, which I think is a RelWithDebugInfo build, with a Release build of WildMIDI. (As of this writing the latest commit is b008d78)

Start the game. It is fine until you try to leave the title menu to start the game, for example by clicking "Test" at the bottom of the screen, or when you exit the game. For information on the code in OpenTESArena involved in the crash and the specific line in the WildMIDI code that the program crashes on, please see the issue over at OpenTESArena I linked to above. It seems it may have something to do with _WM_do_meta_copyright.

[sezero]

Might (or might not) be a double free()??

Can you give us the midi file, so that we can test and reproduce without
having to setup OpenTESArena?

[afritz1]

Thanks for writing this up, @Allofich. Just to add some more information, I think this is just an issue with the Windows build of WildMIDI, and assuming that WildMIDI is calling free() correctly, the only cause of the problem is that there's a mismatch with MSVC libraries being used (MSVCP80.DLL vs. MSVCP140.DLL), causing the application to have problems when trying to free a block on the heap. It's just a shot in the dark though.

This crash also happens when a song finishes in the console, but only when I use WildMIDI's build. When I compile WildMIDI myself with RelWithDebInfo and Visual Studio 2015, the console doesn't crash when a song finishes.

Edit: @sezero, the MIDI files are available in issue #149. It's not just .XMI and .XFM files that cause WildMIDI to crash on free, though (also .MID files converted by WildMIDI).

[sezero]

MSVCP80.DLL and MSVCP140.DLL are not supposed to mix: issue
should be something else. We are either double free()ing, or overwriting
a pointer, or vs2005 is miscompiling something.

I can't crash it on linux by running under valgrind.

Can you make it crash if you build current git master branch with
your own Visual Studio as a Release build?

[Allofich]

Can you make it crash if you build current git master branch with
your own Visual Studio as a Release build?

You mean make a release build of current git master branch of WildMIDI? Yes, it crashes for me, using the built .dll with OpenTESArena.

Also the MIDI I was having a problem with was PERCNTRO.XMI, different from the ones afritz1 linked to, but I tried those (OVERSNOW.XMI, SUNNYDAY.XMI, VISION.XMI) and they have the same problem.

In all cases the crash happens at

wildmidi/src/internal_midi.c

Line 1938 in b008d78

free(mdi->events[i].event_data.data.string);

OpenTESArena calls WildMidi_Close(), and WildMidi_Close() calls _WM_freeMDI() which then crashes at the above line.

[sezero]

I put a debug print in there, like:

diff --git a/src/internal_midi.c b/src/internal_midi.c
index e74c24b..beeb57c 100644
--- a/src/internal_midi.c
+++ b/src/internal_midi.c
@@ -1945,6 +1945,7 @@ void _WM_freeMDI(struct _mdi *mdi) {
                 free(mdi->events[i].event_data.data.string);
             } else if (mdi->events[i].do_event == _WM_do_meta_copyright) {
                 free(mdi->events[i].event_data.data.string);
+                printf("free()d copyright string\n");
             } else if (mdi->events[i].do_event == _WM_do_meta_trackname) {
                 free(mdi->events[i].event_data.data.string);
             } else if (mdi->events[i].do_event == _WM_do_meta_instrumentname) {

With either of OVERSNOW.XMI, SUNNYDAY.XMI and VISION.XMI, I don't ever
hit that print..

[sezero]

@psi29a: can you reproduce this in a simple way?

[sezero]

@Allofich: can you crash your built dll if you play those XMI files
using the newly built wildmidi.exe instead of opentesarena?

[psi29a]

@sezero I'll see if I can repro this on Linux (release/debug builds). I don't have a windows box to test sadly.

[sezero]

My tests were on linux, btw: I failed to reproduce using wildmidi-only.
Hope that you have better luck.

BTW, I pushed db42139: there was a duplicated check in there.
Irrelevant to this issue, but still.

[sezero]

I am not good with C++ so this may sound stupid, but, does setting
mSong to NULL in OpenTESArena after closing it help at all?? E.g.:

diff --git a/OpenTESArena/src/Media/WildMidi.cpp b/OpenTESArena/src/Media/WildMidi.cpp
index dabc096..be019b9 100644
--- a/OpenTESArena/src/Media/WildMidi.cpp
+++ b/OpenTESArena/src/Media/WildMidi.cpp
@@ -34,6 +34,7 @@ namespace
        WildMidiSong::~WildMidiSong()
        {
                WildMidi_Close(mSong);
+               mSong = nullptr;
        }
 
        void WildMidiSong::getFormat(int *sampleRate)

[Allofich]

can you crash your built dll if you play those XMI files
using the newly built wildmidi.exe instead of opentesarena?

I tried it now. I have never used the wildmidi.exe player before so I'm not sure about how to set up .pat sounds for it. However even without those it can silently "play" OVERSNOW.XMI, which I am using as a test. ~~~There is no crash on exit. However for me it does use the line I mentioned above, many times in fact. I have fprintf commands around that line and on closing OVERSNOW.XMI they are printed 21 times.~~~
Edit: Sorry, careless error. I forgot I had commented out the crashing line yesterday. I rebuilt wildmidi.exe with the line restored, and it crashed there, when closing OVERSNOW.XMI.

By the way using the wildmidi.exe from the releases page it complains that the .XMI file is not a MIDI file, but with the one I built from source it can play silently as I described above.

I am not good with C++ so this may sound stupid, but, does setting
mSong to NULL in OpenTESArena after closing it help at all?? E.g.:

I'll try this next.
Edit: Still crashes. As it should, since the crash happens before that line could be reached anyway.

[Allofich]

So anyway, since I have a crash on the same line with a release build of wildmidi.exe when closing OVERSNOW.XMI in Windows7 64-bit, and it seems sezero on Linux does not, it seems to confirm that:

• The problem has a relation to the OS being used
• The problem is in WildMIDI (or something related to being on Windows or non-Linux), not in OpenTESArena

[Allofich]

Did a test. I added fprintf statements for the number of events and the event types in that section of crashing code, like

if (mdi->event_count != 0) {
        for (i = 0; i < mdi->event_count; i++) {
            /* Free up the string event storage */
            fprintf(stdout, "event_count %d", mdi->event_count);
            if (mdi->events[i].do_event == _WM_do_meta_text) {
                fprintf(stdout, "_WM_do_meta_text");
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_text) {
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_copyright) {
                fprintf(stdout, "_WM_do_meta_copyright");
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_trackname) {
                fprintf(stdout, "_WM_do_meta_trackname");
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_instrumentname) {
                fprintf(stdout, "_WM_do_meta_instrumentname");
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_lyric) {
                fprintf(stdout, "_WM_do_meta_lyric");
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_marker) {
                fprintf(stdout, "_WM_do_meta_marker");
                free(mdi->events[i].event_data.data.string);
            } else if (mdi->events[i].do_event == _WM_do_meta_cuepoint) {
                fprintf(stdout, "_WM_do_meta_cuepoint");
                free(mdi->events[i].event_data.data.string);
            }
        }
    }

Result for OVERSNOW.XMI was
event_count 4503_WM_do_meta_copyright then crash.
Result for SUNNYDAY.XMI was
event_count 2425_WM_do_meta_copyright then crash.
Result for VISION.XMI was
event_count 1463_WM_do_meta_copyright then crash.

Looks like it crashes on the first event in each case, which is always _WM_do_meta_copyright.

[sezero]

@Allofich: does the mingw build from wildmidi-0.4.2-win64 (or -win32) release
work properly (wildmidi.exe + libWildMidi.dll) for you, instead of the msvc build?

If it works properly, it has something to do with MSVC?? And it sohould not be
limited to VS2005, because you said you own build using msvc crashes.
(What MSVC version are you using?)

[Allofich]

does the mingw build from wildmidi-0.4.2-win64 (or -win32) release
work properly (wildmidi.exe + libWildMidi.dll) for you, instead of the msvc build?

The mingw build is the one in the main folder (not the msvc folder) when I extract wildmidi-0.3.13-win64.zip, right?

But as I said above, I cannot play the MIDI, because it says ERROR Not a midi file, unlike the wildmidi.exe produced by building from source. How can I play it?

(What MSVC version are you using?)

Visual Studio 2015. Specifically:
Microsoft Visual Studio Community 2015
Version 14.0.25431.01 Update 3