shithub: choc

Info  •  Files  •  Log  •  Branches

Merge pull request #1186 from chocolate-doom/writefilerace2

Clean up of midiproc interface to prevent race conditions.

--- a/midiproc/main.c

+++ b/midiproc/main.c

@@ -119,7 +119,6 @@

 static boolean RegisterSong(const char *filename)

 {

-    UnregisterSong();

     music = Mix_LoadMUS(filename);

     // Remove the temporary MIDI file

@@ -165,9 +164,6 @@

 static boolean MidiPipe_RegisterSong(buffer_reader_t *reader)

 {

-    CHAR buffer[2];

-    DWORD bytes_written;

-

     char *filename = Reader_ReadString(reader);

     if (filename == NULL)

     {

@@ -174,20 +170,12 @@

         return false;

     }

-    if (!RegisterSong(filename))

-    {

-        return false;

-    }

+    return RegisterSong(filename);

+}

-    if (!WriteInt16(buffer, sizeof(buffer),

-                    MIDIPIPE_PACKET_TYPE_REGISTER_SONG_ACK))

-    {

-        return false;

-    }

-

-    WriteFile(midi_process_out, buffer, sizeof(buffer),

-              &bytes_written, NULL);

-

+static boolean MidiPipe_UnregisterSong(buffer_reader_t *reader)

+{

+    UnregisterSong();

     return true;

 }

@@ -222,7 +210,6 @@

 boolean MidiPipe_StopSong()

 {

     StopSong();

-    UnregisterSong();

     return true;

 }

@@ -246,6 +233,8 @@

     {

     case MIDIPIPE_PACKET_TYPE_REGISTER_SONG:

         return MidiPipe_RegisterSong(reader);

+    case MIDIPIPE_PACKET_TYPE_UNREGISTER_SONG:

+        return MidiPipe_UnregisterSong(reader);

     case MIDIPIPE_PACKET_TYPE_SET_VOLUME:

         return MidiPipe_SetVolume(reader);

     case MIDIPIPE_PACKET_TYPE_PLAY_SONG:

@@ -264,6 +253,8 @@

 //

 boolean ParseMessage(buffer_t *buf)

 {

+    CHAR buffer[2];

+    DWORD bytes_written;

     int bytes_read;

     uint16_t command;

     buffer_reader_t *reader = NewReader(buf);

@@ -285,6 +276,15 @@

     bytes_read = Reader_BytesRead(reader);

     DeleteReader(reader);

     Buffer_Shift(buf, bytes_read);

+

+    // Send acknowledgement back that the command has completed.

+    if (!WriteInt16(buffer, sizeof(buffer), MIDIPIPE_PACKET_TYPE_ACK))

+    {

+        goto fail;

+    }

+

+    WriteFile(midi_process_out, buffer, sizeof(buffer),

+              &bytes_written, NULL);

     return true;

--- a/midiproc/proto.h

+++ b/midiproc/proto.h

@@ -20,11 +20,13 @@

 typedef enum {

     MIDIPIPE_PACKET_TYPE_REGISTER_SONG,

-    MIDIPIPE_PACKET_TYPE_REGISTER_SONG_ACK,

+    MIDIPIPE_PACKET_TYPE__DEPRECATED_1,

     MIDIPIPE_PACKET_TYPE_SET_VOLUME,

     MIDIPIPE_PACKET_TYPE_PLAY_SONG,

     MIDIPIPE_PACKET_TYPE_STOP_SONG,

-    MIDIPIPE_PACKET_TYPE_SHUTDOWN

+    MIDIPIPE_PACKET_TYPE_SHUTDOWN,

+    MIDIPIPE_PACKET_TYPE_UNREGISTER_SONG,

+    MIDIPIPE_PACKET_TYPE_ACK,

 } net_midipipe_packet_type_t;

 #endif

--- a/src/i_midipipe.c

+++ b/src/i_midipipe.c

@@ -215,6 +215,19 @@

     *(fp + 1) = '\0';

 }

+static boolean BlockForAck(void)

+{

+    boolean ok;

+    net_packet_t *packet;

+

+    packet = NET_NewPacket(2);

+    NET_WriteInt16(packet, MIDIPIPE_PACKET_TYPE_ACK);

+    ok = ExpectPipe(packet);

+    NET_FreePacket(packet);

+

+    return ok;

+}

+

 //=============================================================================

 //

 // Protocol Commands

@@ -239,6 +252,7 @@

     midi_server_registered = false;

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_RegisterSong failed");

@@ -245,21 +259,37 @@

         return false;

     }

-    packet = NET_NewPacket(2);

-    NET_WriteInt16(packet, MIDIPIPE_PACKET_TYPE_REGISTER_SONG_ACK);

-    ok = ExpectPipe(packet);

+    midi_server_registered = true;

+

+    DEBUGOUT("I_MidiPipe_RegisterSong succeeded");

+    return true;

+}

+

+//

+// I_MidiPipe_UnregisterSong

+//

+// Tells the MIDI subprocess to unload the current song.

+//

+void I_MidiPipe_UnregisterSong(void)

+{

+    boolean ok;

+    net_packet_t *packet;

+

+    packet = NET_NewPacket(64);

+    NET_WriteInt16(packet, MIDIPIPE_PACKET_TYPE_UNREGISTER_SONG);

+    ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     if (!ok)

     {

-        DEBUGOUT("I_MidiPipe_RegisterSong ack failed");

-        return false;

+        DEBUGOUT("I_MidiPipe_UnregisterSong failed");

+        return;

     }

-    midi_server_registered = true;

+    midi_server_registered = false;

-    DEBUGOUT("I_MidiPipe_RegisterSong succeeded");

-    return true;

+    DEBUGOUT("I_MidiPipe_UnregisterSong succeeded");

 }

 //

@@ -278,6 +308,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_SetVolume failed");

@@ -303,6 +334,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_PlaySong failed");

@@ -329,6 +361,7 @@

     midi_server_registered = false;

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_StopSong failed");

@@ -353,6 +386,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     FreePipes();

     midi_server_initialized = false;

--- a/src/i_midipipe.h

+++ b/src/i_midipipe.h

@@ -29,6 +29,7 @@

 extern boolean midi_server_registered;

 boolean I_MidiPipe_RegisterSong(char *filename);

+void I_MidiPipe_UnregisterSong(void);

 void I_MidiPipe_SetVolume(int vol);

 void I_MidiPipe_PlaySong(int loops);

 void I_MidiPipe_StopSong();

--- a/src/i_sdlmusic.c

+++ b/src/i_sdlmusic.c

@@ -345,12 +345,19 @@

         return;

     }

-    if (handle == NULL)

+#if defined(_WIN32)

+    if (midi_server_registered)

     {

-        return;

+        I_MidiPipe_StopSong();

     }

-

-    Mix_FreeMusic(music);

+    else

+#endif

+    {

+        if (handle != NULL)

+        {

+            Mix_FreeMusic(music);

+        }

+    }

 }

 // Determine whether memory block is a .mid file