shithub: choc

Info  •  Files  •  Log  •  Branches

midiproc: Acknowledge all commands on completion.

Race conditions can occur if we do not know for sure whether a
midiproc command has completed yet or not. So expand on the ACK packet
type already sent for song registration and send acks for all commands
sent by the caller.

Second part of a fix for #963.

--- a/midiproc/main.c

+++ b/midiproc/main.c

@@ -164,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)

     {

@@ -173,21 +170,7 @@

         return false;

     }

-    if (!RegisterSong(filename))

-    {

-        return false;

-    }

-

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

-                    MIDIPIPE_PACKET_TYPE_REGISTER_SONG_ACK))

-    {

-        return false;

-    }

-

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

-              &bytes_written, NULL);

-

-    return true;

+    return RegisterSong(filename);

 }

 static boolean MidiPipe_UnregisterSong(buffer_reader_t *reader)

@@ -270,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);

@@ -291,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

@@ -26,6 +26,7 @@

     MIDIPIPE_PACKET_TYPE_STOP_SONG,

     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_REGISTER_SONG_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,17 +259,6 @@

         return false;

     }

-    packet = NET_NewPacket(2);

-    NET_WriteInt16(packet, MIDIPIPE_PACKET_TYPE_REGISTER_SONG_ACK);

-    ok = ExpectPipe(packet);

-    NET_FreePacket(packet);

-

-    if (!ok)

-    {

-        DEBUGOUT("I_MidiPipe_RegisterSong ack failed");

-        return false;

-    }

-

     midi_server_registered = true;

     DEBUGOUT("I_MidiPipe_RegisterSong succeeded");

@@ -277,6 +280,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_UnregisterSong failed");

@@ -304,6 +308,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_SetVolume failed");

@@ -329,6 +334,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_PlaySong failed");

@@ -355,6 +361,7 @@

     midi_server_registered = false;

+    ok = ok && BlockForAck();

     if (!ok)

     {

         DEBUGOUT("I_MidiPipe_StopSong failed");

@@ -379,6 +386,7 @@

     ok = WritePipe(packet);

     NET_FreePacket(packet);

+    ok = ok && BlockForAck();

     FreePipes();

     midi_server_initialized = false;