8074211: javax.sound.midi: Error with send System Exclusive messages of different length

AlecJY · prrace · commit e47cf611c949 · 2023-11-21T19:26:49.000Z
8250667: MIDI sysex over USB scrambled when reply length matches previous message

Reviewed-by: prr
diff --git a/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiIn.cpp b/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiIn.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -301,7 +301,7 @@ INT32 prepareBuffers(MidiDeviceHandle* handle) {
     }
     sysex = (SysExQueue*) handle->longBuffers;
     for (i = 0; i<sysex->count; i++) {
-        MIDIHDR* hdr = &(sysex->header[i]);
+        MIDIHDR* hdr = &(sysex->headerInfo[i].header);
         midiInPrepareHeader((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));
         err = midiInAddBuffer((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));
     }
@@ -320,7 +320,7 @@ INT32 unprepareBuffers(MidiDeviceHandle* handle) {
     }
     sysex = (SysExQueue*) handle->longBuffers;
     for (i = 0; i<sysex->count; i++) {
-        err = midiInUnprepareHeader((HMIDIIN) handle->deviceHandle, &(sysex->header[i]), sizeof(MIDIHDR));
+        err = midiInUnprepareHeader((HMIDIIN) handle->deviceHandle, &(sysex->headerInfo[i].header), sizeof(MIDIHDR));
     }
     MIDIIN_CHECK_ERROR;
     return (INT32) err;
@@ -502,7 +502,7 @@ void MIDI_IN_ReleaseMessage(MidiDeviceHandle* handle, MidiMessage* msg) {
     }
     sysex = (SysExQueue*) handle->longBuffers;
     if (msg->type == LONG_MESSAGE && sysex) {
-        MIDIHDR* hdr = &(sysex->header[msg->data.l.index]);
+        MIDIHDR* hdr = &(sysex->headerInfo[msg->data.l.index].header);
         //fprintf(stdout, "ReleaseMessage index %d\n", msg->data.l.index); fflush(stdout);
         hdr->dwBytesRecorded = 0;
         midiInAddBuffer((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));
diff --git a/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiOut.c b/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiOut.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -161,7 +161,7 @@ INT32 unprepareLongBuffers(MidiDeviceHandle* handle) {
     }
     sysex = (SysExQueue*) handle->longBuffers;
     for (i = 0; i<sysex->count; i++) {
-        MIDIHDR* hdr = &(sysex->header[i]);
+        MIDIHDR* hdr = &(sysex->headerInfo[i].header);
         if (hdr->dwFlags) {
             err = midiOutUnprepareHeader((HMIDIOUT) handle->deviceHandle, hdr, sizeof(MIDIHDR));
         }
@@ -170,8 +170,9 @@ INT32 unprepareLongBuffers(MidiDeviceHandle* handle) {
     return (INT32) err;
 }
 
-INT32 freeLongBuffer(MIDIHDR* hdr, HMIDIOUT deviceHandle, INT32 minToLeaveData) {
+INT32 freeLongBuffer(MidiHeaderInfo* info, HMIDIOUT deviceHandle, INT32 minToLeaveData) {
     MMRESULT err = MMSYSERR_NOERROR;
+    MIDIHDR* hdr = &(info->header);
 
     if (!hdr) {
         ERROR0("MIDI_OUT_freeLongBuffer: hdr == NULL\n");
@@ -180,10 +181,11 @@ INT32 freeLongBuffer(MIDIHDR* hdr, HMIDIOUT deviceHandle, INT32 minToLeaveData)
     if (hdr->dwFlags && deviceHandle) {
         err = midiOutUnprepareHeader(deviceHandle, hdr, sizeof(MIDIHDR));
     }
-    if (hdr->lpData && (((INT32) hdr->dwBufferLength) < minToLeaveData || minToLeaveData < 0)) {
+    if (hdr->lpData && (info->bufferLength < minToLeaveData || minToLeaveData < 0)) {
         free(hdr->lpData);
         hdr->lpData=NULL;
         hdr->dwBufferLength=0;
+        info->bufferLength=0;
     }
     hdr->dwBytesRecorded=0;
     hdr->dwFlags=0;
@@ -201,7 +203,7 @@ INT32 freeLongBuffers(MidiDeviceHandle* handle) {
     }
     sysex = (SysExQueue*) handle->longBuffers;
     for (i = 0; i<sysex->count; i++) {
-        err = freeLongBuffer(&(sysex->header[i]), (HMIDIOUT) handle->deviceHandle, -1);
+        err = freeLongBuffer(&(sysex->headerInfo[i]), (HMIDIOUT) handle->deviceHandle, -1);
     }
     MIDIOUT_CHECK_ERROR;
     return (INT32) err;
@@ -352,6 +354,7 @@ INT32 MIDI_OUT_SendShortMessage(MidiDeviceHandle* handle, UINT32 packedMsg, UINT
 INT32 MIDI_OUT_SendLongMessage(MidiDeviceHandle* handle, UBYTE* data, UINT32 size, UINT32 timestamp) {
     MMRESULT err;
     SysExQueue* sysex;
+    MidiHeaderInfo* info = NULL;
     MIDIHDR* hdr = NULL;
     INT32 remainingSize;
     int i;
@@ -378,10 +381,12 @@ INT32 MIDI_OUT_SendLongMessage(MidiDeviceHandle* handle, UBYTE* data, UINT32 siz
         while (!hdr && handle->platformData) {
             /* find a non-queued header */
             for (i = 0; i < sysex->count; i++) {
-                hdr = &(sysex->header[i]);
+                info = &(sysex->headerInfo[i]);
+                hdr = &(info->header);
                 if ((hdr->dwFlags & MHDR_DONE) || (hdr->dwFlags == 0)) {
                     break;
                 }
+                info = NULL;
                 hdr = NULL;
             }
             /* wait for a buffer to free up */
@@ -404,22 +409,26 @@ INT32 MIDI_OUT_SendLongMessage(MidiDeviceHandle* handle, UBYTE* data, UINT32 siz
         }
 
         TRACE2("-> sending %d bytes with buffer index=%d\n", (int) size, (int) hdr->dwUser);
-        freeLongBuffer(hdr, handle->deviceHandle, (INT32) size);
+        freeLongBuffer(info, handle->deviceHandle, (INT32) size);
         if (hdr->lpData == NULL) {
             hdr->lpData = malloc(size);
-            hdr->dwBufferLength = size;
+            info->bufferLength = size;
         }
+        // Because midiOutLongMsg() ignores dwBytesRecorded, set both
+        // dwBufferLength to the size of the data. The actual buffer
+        // size is recorded in info->bufferLength.
+        hdr->dwBufferLength = size;
         hdr->dwBytesRecorded = size;
         memcpy(hdr->lpData, data, size);
         err = midiOutPrepareHeader((HMIDIOUT) handle->deviceHandle, hdr, sizeof(MIDIHDR));
         if (err != MMSYSERR_NOERROR) {
-            freeLongBuffer(hdr, handle->deviceHandle, -1);
+            freeLongBuffer(info, handle->deviceHandle, -1);
             MIDIOUT_CHECK_ERROR;
             return (INT32) err;
         }
         err = midiOutLongMsg((HMIDIOUT) handle->deviceHandle, hdr, sizeof(MIDIHDR));
         if (err != MMSYSERR_NOERROR) {
-            freeLongBuffer(hdr, handle->deviceHandle, -1);
+            freeLongBuffer(info, handle->deviceHandle, -1);
             ERROR0("ERROR: MIDI_OUT_SendLongMessage: midiOutLongMsg returned error:\n");
             MIDIOUT_CHECK_ERROR;
             return (INT32) err;
diff --git a/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_Util.c b/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_Util.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2007, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -89,7 +89,7 @@ int MIDI_WinCreateLongBufferQueue(MidiDeviceHandle* handle, int count, int size,
     SysExQueue* sysex;
     int i;
     UBYTE* dataPtr;
-    int structSize = sizeof(SysExQueue) + ((count - 1) * sizeof(MIDIHDR));
+    int structSize = sizeof(SysExQueue) + ((count - 1) * sizeof(MidiHeaderInfo));
 
     sysex = (SysExQueue*) malloc(structSize);
     if (!sysex) return FALSE;
@@ -112,10 +112,11 @@ int MIDI_WinCreateLongBufferQueue(MidiDeviceHandle* handle, int count, int size,
     // set up headers
     dataPtr = preAllocatedMem;
     for (i=0; i<count; i++) {
-        sysex->header[i].lpData = dataPtr;
-        sysex->header[i].dwBufferLength = size;
+        sysex->headerInfo[i].header.lpData = dataPtr;
+        sysex->headerInfo[i].header.dwBufferLength = size;
+        sysex->headerInfo[i].bufferLength = size;
         // user data is the index of the buffer
-        sysex->header[i].dwUser = (DWORD) i;
+        sysex->headerInfo[i].header.dwUser = (DWORD) i;
         dataPtr += size;
     }
     return TRUE;
diff --git a/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_Util.h b/src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_Util.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2007, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -46,12 +46,17 @@
 
 #include "PlatformMidi.h"
 
+typedef struct tag_MidiHeaderInfo {
+    MIDIHDR header;     // Windows specific structure to hold meta info
+    INT32 bufferLength; // the actual length of the buffer in MIDIHDR
+} MidiHeaderInfo;
+
 typedef struct tag_SysExQueue {
-    int count;         // number of sys ex headers
-    int size;          // data size per sys ex header
-    int ownsLinearMem; // true when linearMem is to be disposed
-    UBYTE* linearMem;  // where the actual sys ex data is, count*size bytes
-    MIDIHDR header[1]; // Windows specific structure to hold meta info
+    int count;                    // number of sys ex headers
+    int size;                     // data size per sys ex header
+    int ownsLinearMem;            // true when linearMem is to be disposed
+    UBYTE* linearMem;             // where the actual sys ex data is, count*size bytes
+    MidiHeaderInfo headerInfo[1]; // a structure to hold MIDIHDR and the actual buffer length
 } SysExQueue;
 
 /* set the startTime field in MidiDeviceHandle */
diff --git a/test/jdk/javax/sound/midi/SysexMessage/SendRawSysexMessage.java b/test/jdk/javax/sound/midi/SysexMessage/SendRawSysexMessage.java
@@ -34,7 +34,7 @@
 
 /**
  * @test
- * @bug 8237495 8301310
+ * @bug 8074211 8237495 8301310
  * @summary fail with memory errors when asked to send a sysex message starting
  *          with 0xF7
  */
@@ -114,6 +114,11 @@ private static void test(MidiDevice.Info info) throws Exception {
                         (byte) SPECIAL_SYSTEM_EXCLUSIVE}), -1);
                 System.err.println("note off");
                 r.send(new ShortMessage(ShortMessage.NOTE_OFF, 5, 5), -1);
+                // The three parts of the sysex below are added for
+                // JDK-8301310, but it can also used to test JDK-8074211.
+                // However, The testcase does not fail when JDK-8074211 occurs.
+                // It's recommended to setup a loopback MIDI device then check
+                // whether the sysex received is the same as the testcase.
                 System.err.println("sysex part 1 of 3");
                 r.send(new SysexMessage(new byte[]{
                         (byte) SYSTEM_EXCLUSIVE, 0x7D, 0x01, 0x02}, 4), -1);

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* This code is free software; you can redistribute it and/or modify it`
`@@ -301,7 +301,7 @@ INT32 prepareBuffers(MidiDeviceHandle* handle) {`
`301`	`301`	`}`
`302`	`302`	`sysex = (SysExQueue*) handle->longBuffers;`
`303`	`303`	`for (i = 0; i<sysex->count; i++) {`
`304`		`- MIDIHDR* hdr = &(sysex->header[i]);`
	`304`	`+ MIDIHDR* hdr = &(sysex->headerInfo[i].header);`
`305`	`305`	`midiInPrepareHeader((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));`
`306`	`306`	`err = midiInAddBuffer((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));`
`307`	`307`	`}`
`@@ -320,7 +320,7 @@ INT32 unprepareBuffers(MidiDeviceHandle* handle) {`
`320`	`320`	`}`
`321`	`321`	`sysex = (SysExQueue*) handle->longBuffers;`
`322`	`322`	`for (i = 0; i<sysex->count; i++) {`
`323`		`- err = midiInUnprepareHeader((HMIDIIN) handle->deviceHandle, &(sysex->header[i]), sizeof(MIDIHDR));`
	`323`	`+ err = midiInUnprepareHeader((HMIDIIN) handle->deviceHandle, &(sysex->headerInfo[i].header), sizeof(MIDIHDR));`
`324`	`324`	`}`
`325`	`325`	`MIDIIN_CHECK_ERROR;`
`326`	`326`	`return (INT32) err;`
`@@ -502,7 +502,7 @@ void MIDI_IN_ReleaseMessage(MidiDeviceHandle* handle, MidiMessage* msg) {`
`502`	`502`	`}`
`503`	`503`	`sysex = (SysExQueue*) handle->longBuffers;`
`504`	`504`	`if (msg->type == LONG_MESSAGE && sysex) {`
`505`		`- MIDIHDR* hdr = &(sysex->header[msg->data.l.index]);`
	`505`	`+ MIDIHDR* hdr = &(sysex->headerInfo[msg->data.l.index].header);`
`506`	`506`	`//fprintf(stdout, "ReleaseMessage index %d\n", msg->data.l.index); fflush(stdout);`
`507`	`507`	`hdr->dwBytesRecorded = 0;`
`508`	`508`	`midiInAddBuffer((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));`