Avoid double-freeing memory, issue DerekCook#19

Make sure CoreMidiSource and CoreMidiDestination are protected against
being opened or closed multiple times.

Also improves conformance with the Java MIDI SPI spec by returning
empty lists when there are no receivers/transmitters rather than NULL.

CHANGELOG.md

@@ -6,7 +6,11 @@ This change log follows the conventions of
 
 ## [Unreleased][unreleased]
 
-Nothing so far.
+### Fixed
+
+- Calling `close()` more than once on a `CoreMidiSource` would lead to
+  a crash as the same block of memory is attempted to be freed more
+  than once.
 
 ## [1.0] - 2017-05-14
 

CoreMIDI4J/Native/CoreMidi4J/CoreMidiInputPort.cpp

@@ -252,9 +252,11 @@ JNIEXPORT void JNICALL Java_uk_co_xfactorylibrarians_coremidi4j_CoreMidiInputPor
   // Disconnect this input port from the end port
   status = MIDIPortDisconnectSource(inputPortReference, sourceEndPointReference);
 
-  // Delete the globsal reference to the Java CoreMidiInputPort object
+  // Delete the global reference to the Java CoreMidiInputPort object
   env->DeleteGlobalRef(((MIDI_CALLBACK_PARAMETERS *) memoryReference)->object);
 
+  //std::cout << "Trying to release " << memoryReference << std::endl;
+
   // Release the memory block that Java_uk_co_xfactorylibrarians_coremidi4j_CoreMidiInputPort_midiPortConnectSource allocated
   free((void *) memoryReference);
 

CoreMIDI4J/pom.xml

@@ -6,7 +6,7 @@
 
   <groupId>uk.co.xfactory-librarians</groupId>
   <artifactId>coremidi4j</artifactId>
-  <version>1.0</version>
+  <version>1.1-SNAPSHOT</version>
 
   <packaging>jar</packaging>
 

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDestination.java

@@ -14,6 +14,7 @@
 
 package uk.co.xfactorylibrarians.coremidi4j;
 
+import java.util.Collections;
 import java.util.List;
 
 import javax.sound.midi.MidiDevice;
@@ -191,15 +192,15 @@ public Receiver getReceiver() throws MidiUnavailableException {
    * 
    * @see javax.sound.midi.MidiDevice#getReceivers()
    * 
-   * @return NULL - we do not maintain a list of receivers 
+   * @return an empty list - we do not maintain a list of receivers
    * 
    */
 
   @Override
   public List<Receiver> getReceivers() {
 
     // We do not maintain a list of receivers, as they tend to be transitory context
-    return null;
+    return Collections.emptyList();
 
   }
 
@@ -232,7 +233,7 @@ public Transmitter getTransmitter() throws MidiUnavailableException {
   public List<Transmitter> getTransmitters() {
 
     // A CoreMIDI Destination has no transmitters
-    return null;
+    return Collections.emptyList();
 
   }
 

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiSource.java

@@ -18,6 +18,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.sound.midi.InvalidMidiDataException;
 import javax.sound.midi.MidiDevice;
@@ -36,7 +37,7 @@
 public class CoreMidiSource implements MidiDevice {
 
   private final CoreMidiDeviceInfo info;
-  private boolean isOpen;
+  private final AtomicBoolean isOpen;
   private CoreMidiInputPort input = null;
   private final List<Transmitter> transmitters;
 
@@ -52,15 +53,12 @@ public class CoreMidiSource implements MidiDevice {
    * Default constructor.
    *
    * @param info a CoreMidiDeviceInfo object providing details of the MIDI interface
-   * 
-   * @throws CoreMidiException
-   * 
    */
 
-  CoreMidiSource(CoreMidiDeviceInfo info) throws CoreMidiException {
+  CoreMidiSource(CoreMidiDeviceInfo info) {
 
     this.info = info;
-    this.isOpen = false;
+    this.isOpen = new AtomicBoolean(false);
     transmitters = new ArrayList<Transmitter>();
 
   }
@@ -89,26 +87,29 @@ public Info getDeviceInfo() {
   @Override
   public void open() throws MidiUnavailableException {
 
-    try {
+    if (isOpen.compareAndSet(false, true)) {
 
-      // Create the input port if not already created
-      if (this.input == null) {
+      try {
 
-        this.input = CoreMidiDeviceProvider.getMIDIClient().inputPortCreate("Core Midi Provider Input");
+        // Create the input port if not already created
+        if (this.input == null) {
 
-      }
+          this.input = CoreMidiDeviceProvider.getMIDIClient().inputPortCreate("Core Midi Provider Input");
+
+        }
 
-      // And connect to it
-      this.input.connectSource(this);
-      isOpen = true;
-
-      // Get the system time in microseconds
-      startTime = this.getMicroSecondTime();
+        // And connect to it
+        this.input.connectSource(this);
 
-    } catch (CoreMidiException e) {
+        // Get the system time in microseconds
+        startTime = this.getMicroSecondTime();
 
-      e.printStackTrace();
-      throw new MidiUnavailableException(e.getMessage());
+      } catch (CoreMidiException e) {
+
+        e.printStackTrace();
+        throw new MidiUnavailableException(e.getMessage());
+
+      }
 
     }
 
@@ -122,30 +123,37 @@ public void open() throws MidiUnavailableException {
   @Override
   public void close() {
 
-    try {
+    if (isOpen.compareAndSet(true, false)) {
 
-      // If the port is created then disconnect from it
-      if (this.input != null) {
+      try {
 
-        this.input.disconnectSource(this);
+        // If the port is created then disconnect from it
+        if (this.input != null) {
 
-      }
+          try {
 
-      // Clear the transmitter list
-      synchronized (transmitters) {
+            this.input.disconnectSource(this);
 
-        transmitters.clear();
+          } finally {
 
-      }
+            this.input = null;
+
+          }
+
+        }
 
-    } catch (CoreMidiException e) {
+        // Clear the transmitter list
+        synchronized (transmitters) {
 
-      e.printStackTrace();
+          transmitters.clear();
 
-    } finally {
+        }
+
+      } catch (CoreMidiException e) {
+
+        e.printStackTrace();
 
-      // Reset the context data
-      isOpen = false;
+      }
 
     }
 
@@ -163,7 +171,7 @@ public void close() {
   @Override
   public boolean isOpen() {
 
-    return isOpen;
+    return isOpen.get();
 
   }
 
@@ -237,7 +245,7 @@ public Receiver getReceiver() throws MidiUnavailableException {
   /**
    * Gets a list of receivers connected to the device
    *
-   * @return NULL - we do not maintain a list of receivers
+   * @return an empty list - we do not maintain a list of receivers
    * 
    * @see javax.sound.midi.MidiDevice#getReceivers()
    * 
@@ -247,12 +255,12 @@ public Receiver getReceiver() throws MidiUnavailableException {
   public List<Receiver> getReceivers() {
 
     // A CoreMidiSource has no receivers
-    return null;
+    return Collections.emptyList();
 
   }
 
   /**
-   * Gets a transmitter for this device (which is also added to the internal list
+   * Gets a transmitter for this device (which is also added to the internal list)
    *
    * @return a transmitter for this device
    * 
@@ -293,11 +301,9 @@ public List<Transmitter> getTransmitters() {
     // Create and return a list of transmitters
     synchronized (transmitters) {
 
-      final List<Transmitter> list = new ArrayList<Transmitter>();
-
-      list.addAll(transmitters);
+      final List<Transmitter> list = new ArrayList<Transmitter>(transmitters);
 
-      return list;
+      return Collections.unmodifiableList(list);
 
     }