Game crashes when running with MIDI

[Allofich]

If I run the game with MIDI active, it crashes when I click "Start New Game."

Specifically, if I have eawpats.zip extracted into the Data folder, the game starts and plays MIDI, but crashes when I click Start New Game. If I don't have eawpats in the Data folder, the game doesn't crash.

[Allofich]

The game also crashes on exit when I'm using eawpats.

[afritz1]

You're on Windows, right? I remember having similar issues. It might be caused by using the wrong build of wildmidi_dynamic.dll. I think I fixed it by building the debug version of WildMIDI and using the .dll that came from that. @psi29a, any other advice?

[Allofich]

Yes, I'm on Windows 7, 64-bit. I'll try doing the same thing you did.

[Allofich]

Yes, that worked. And then I built the Release build of wildmidi and used that, and the crash returned. So, back to the Debug build.

[psi29a]

Nice find. You building with MSVC? Have you tried using our binaries from WildMIDI 0.4.1?

[afritz1]

Yeah, I'm using Visual Studio 2015. I set up CMake to build OpenTESArena with the wildmidi-0.4.1-win64/msvc folder just now, and it runs fine in Visual Studio, but if I run the executable itself with wildmidi_dynamic.dll in the directory, then it crashes after clicking "Start New Game", just like with Allofich's crash.

If I build and use the debug version of wildmidi_dynamic.dll with the OpenTESArena executable, it doesn't crash.

[Allofich]

I also use MSVC 2015. From the binaries, do you mean wildmidi.exe and wildmidi-static.exe in the msvc folder of WildMIDI 0.4.1? I tried it, and at first it didn't start because of not finding wildmidi.cfg. I copied that into the folder and tried it on a midi, and while the program ran, I got no sound. It was a Daggerfall midi and maybe it's not supported? And, it crashes when I exit the program. So, either I'm not using it right or there are some problems with it.

[afritz1]

I just realized that all of my Windows releases contain the debug .dll of WildMIDI which requires vcruntime140d.dll (something the typical user doesn't need on their computer. I didn't notice this in the dependency walker until now, either). I've also accidentally been mixing different versions of wildmidi_dynamic.dll with wildmidi_dynamic.lib, but that doesn't seem to be what causes the crash on "Start New Game". If I build the .dll and .lib myself then it seems to work fine.

I'm not sure how the WildMIDI releases could fix this other than maybe linking against a newer MSVC runtime (like MSVCR120.dll instead of MSVCR80.dll), because using the .dll that comes with the release causes the crash.

[Allofich]

I tried the wildmidi_dynamic.dll packaged with the latest WildMIDI release, 0.4.2, but it still crashes, so I'm still using the debug version of the dll, which doesn't crash.

[psi29a]

Can anyone run this in a debugger and pinpoint where and when the crash happens?

[afritz1]

For the test, I built OpenTESArena with:

• CMake 3.5.2
• Visual Studio 2015
• wildmidi-0.4.1-win64 msvc folder (from April 28th, 2017)

The debugger stops with an error message about "A heap has been corrupted" in ntdll.dll after calling WildMidi_Close(midi). The crash happens when closing a MIDI song.

In CMake, I selected the msvc folder for WILDMIDI_INCLUDE_DIR and wildmidi_dynamic.lib for WILDMIDI_LIBRARY.

This error does not occur when I build WildMIDI myself and use its wildmidi_dynamic.lib and wildmidi_dynamic.dll (built with Visual Studio 2013 x64 in RelWithDebInfo mode). That's the one I've been distributing with releases.

[Allofich]

I don't know if this is an acceptable solution/workaround, but commenting out
WildMidi_Close(mSong);
in
WildMidiSong::~WildMidiSong()

allows me to use the release version of the WildMIDI .dll without crashing when I start the game.
There was then a crash on exit, but commenting out
if (sInitState >= 0) WildMidi_Shutdown();
in
WildMidiDevice::~WildMidiDevice()
solved that, and I'm able to start the game, load up different levels, and exit without crashing. Is calling close and shutdown in those destructors correct procedure for using WildMIDI?

[afritz1]

Here is the WildMIDI source code for WildMidi_Close(). It appears to use free(), so unless we want memory leaks, we can't comment it out.

Still not sure why there's that crash in WildMidi_Close(). My best guess is that the MSVC runtime used with wildmidi_dynamic.dll (MSVCR80.DLL) is not compatible with the newer one (MSVCP140.DLL), so maybe the two runtimes have different behavior on the heap or something, causing a mix-up when trying to free a block.

[Allofich]

Yeah I thought removing those could result in memory leaks but also wondered if maybe they're trying to free resources that were already freed somehow. psi29a, does it look to you like OpenTESArena is doing things correctly and this is most likely a problem on the WildMIDI side?

[Allofich]

FWIW, commenting out the statements as mentioned above, using a release build of wildmidi_dynamic.dll and running with Dr. Memory, a program that's supposed to monitor for and catch leaks, it didn't complain about any leaks.

When running with the release build .dll with the statements not commented out and using Dr. Memory, the program doesn't crash like it does without Dr. Memory. It gave the following report information at two points where the program would normally have crashed (when loading a level from the main menu and when exiting the program.)

Loading a level

Error #2: INVALID HEAP ARGUMENT to free 0x000000000000003c
# 0 replace_free                                 [d:\drmemory_package\common\alloc_replace.c:2706]
# 1 KERNEL32.dll!HeapFree                       +0x9      (0x0000000077701a0a <KERNEL32.dll+0x21a0a>)
# 2 wildmidi_dynamic.dll!WildMidi_SongSeek      +0x680a   (0x000007fef1ff9c7b <wildmidi_dynamic.dll+0x9c7b>)
# 3 std::_Func_class<>::operator()               [c:\program files (x86)\microsoft visual studio 14.0\vc\include\functional:279]
# 4 Button<>::click                              [d:\github\opentesarena\opentesarena\src\interface\button.h:97]
# 5 MainMenuPanel::handleEvent                   [d:\github\opentesarena\opentesarena\src\interface\mainmenupanel.cpp:720]
# 6 Game::handleEvents                           [d:\github\opentesarena\opentesarena\src\game\game.cpp:341]
# 7 Game::loop                                   [d:\github\opentesarena\opentesarena\src\game\game.cpp:448]
# 8 SDL_main                                     [d:\github\opentesarena\opentesarena\src\main.cpp:14]
# 9 main_utf8                                    [c:\projects\sdl\src\main\windows\sdl_windows_main.c:126]
#10 main                                         [c:\projects\sdl\src\main\windows\sdl_windows_main.c:134]

Exiting the program

Error #4: INVALID HEAP ARGUMENT to free 0x000000000000003c
# 0 replace_free                      [d:\drmemory_package\common\alloc_replace.c:2706]
# 1 MSVCP140D.dll!_Mtxunlock          [f:\dd\vctools\crt\crtw32\stdcpp\xmtx.c:31]
# 2 wildmidi_dynamic.dll!WildMidi_SongSeek+0x680a   (0x000007fef1ff9c7b <wildmidi_dynamic.dll+0x9c7b>)
# 3 KERNEL32.dll!BaseThreadInitThunk +0xc      (0x00000000776f59cd <KERNEL32.dll+0x159cd>)

[Allofich]

An update.

First, I tried a Release build of the latest WildMidi, but the problem remains.

I tracked down to the exact line where the crash occurs.
https://github.com/Mindwerks/wildmidi/blob/b008d78897ef310f4b83fa069e39137a93980a9f/src/internal_midi.c#L1938

If you comment out this line, no crash will occur.

[psi29a]

Nice work @Allofich
Chris has also found a few bugs and has fixed them recently in Wildmidi, they currently sit in master. That might have fixed it, otherwise please file a bug/issue on the Wildmidi tracker on how to reproduce this issue.

[Allofich]

Hi psi29a, I tried with the latest master just today, so it's not fixed. I'll go ahead and add a bug report, though.

[psi29a]

Thanks, I'll also have a deep dive into this around lunch. I'm wondering if this platform specific, if not then we can try to reproduce this on Linux.

[sezero]

This issue is fixed in willdmidi master branch.

[Allofich]

Yes, I agree. I will go ahead and do so. afritz1 if you want it back open for any reason go ahead and re-open.