foxygit / doom Log in
commit 7edf7d4da58f8feb67e34007ae4e075907b96102
Author:     Roman Fomin <rfomin@gmail.com>
AuthorDate: Mon Jun 19 11:25:06 2023 +0700
Commit:     Roman Fomin <rfomin@gmail.com>
CommitDate: Mon Jun 19 11:25:06 2023 +0700

    avoid midiOutUnprepareHeader() call, wait only 3 seconds for player thread

    midiOutUnprepareHeader() has a memory bug reported by the AddressSanitizer tool.
    Increase the initial buffer size and don't free the buffer on exit to avoid
    calling midiOutUnprepareHeader().
---
 src/i_winmusic.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/src/i_winmusic.c b/src/i_winmusic.c
index 3f00bdf3..c6f94fee 100644
--- a/src/i_winmusic.c
+++ b/src/i_winmusic.c
@@ -125,7 +125,9 @@ typedef struct

 static win_midi_song_t song;

-#define BUFFER_INITIAL_SIZE 1024
+#define BUFFER_INITIAL_SIZE 8192
+
+#define PLAYER_THREAD_WAIT_TIME 3000

 typedef struct
 {
@@ -186,6 +188,10 @@ static void AllocateBuffer(const unsigned int size)

     if (buffer.data)
     {
+        // Windows doesn't always immediately clear the MHDR_INQUEUE flag, even
+        // after midiStreamStop() is called. There doesn't seem to be any side
+        // effect to just forcing the flag off.
+        hdr->dwFlags &= ~MHDR_INQUEUE;
         mmr = midiOutUnprepareHeader((HMIDIOUT)hMidiStream, hdr, sizeof(MIDIHDR));
         if (mmr != MMSYSERR_NOERROR)
         {
@@ -1425,7 +1431,7 @@ static void I_WIN_StopSong(void)
     }

     SetEvent(hExitEvent);
-    WaitForSingleObject(hPlayerThread, INFINITE);
+    WaitForSingleObject(hPlayerThread, PLAYER_THREAD_WAIT_TIME);
     CloseHandle(hPlayerThread);
     hPlayerThread = NULL;

@@ -1634,30 +1640,15 @@ static void I_WIN_ShutdownMusic(void)
     {
         MidiError("midiStreamRestart", mmr);
     }
-    WaitForSingleObject(hBufferReturnEvent, INFINITE);
+    WaitForSingleObject(hBufferReturnEvent, PLAYER_THREAD_WAIT_TIME);
     mmr = midiStreamStop(hMidiStream);
     if (mmr != MMSYSERR_NOERROR)
     {
         MidiError("midiStreamStop", mmr);
     }

-    if (buffer.data)
-    {
-        // Windows doesn't always immediately clear the MHDR_INQUEUE flag, even
-        // after midiStreamStop() is called. There doesn't seem to be any side
-        // effect to just forcing the flag off.
-        MidiStreamHdr.dwFlags &= ~MHDR_INQUEUE;
-        mmr = midiOutUnprepareHeader((HMIDIOUT)hMidiStream, &MidiStreamHdr,
-                                     sizeof(MIDIHDR));
-        if (mmr != MMSYSERR_NOERROR)
-        {
-            MidiError("midiOutUnprepareHeader", mmr);
-        }
-        free(buffer.data);
-        buffer.data = NULL;
-        buffer.size = 0;
-        buffer.position = 0;
-    }
+    // Don't free the buffer to avoid calling midiOutUnprepareHeader() which
+    // contains a memory error (detected by ASan).

     mmr = midiStreamClose(hMidiStream);
     if (mmr != MMSYSERR_NOERROR)