MIDI Manager changes:

Add CloseGuard support to MidiDevice and MidiDeviceServer
Make MidiDevice.close() thread safe
Make non-subclassable API classes final
Other misc cleanup

Change-Id: I7a5d31b06b8c2403cfbc5597c5c1395f0ac90194

7 files changed, 112 insertions, 53 deletions

diff --git a/media/java/android/media/midi/MidiDevice.java b/media/java/android/media/midi/MidiDevice.java
index 1a39485..af0737d 100644
--- a/media/java/android/media/midi/MidiDevice.java
+++ b/media/java/android/media/midi/MidiDevice.java
@@ -24,11 +24,13 @@ import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.util.Log;
 
+import dalvik.system.CloseGuard;
+
 import java.io.Closeable;
 import java.io.IOException;
 
 /**
- * This class is used for sending and receiving data to and from an MIDI device
+ * This class is used for sending and receiving data to and from a MIDI device
  * Instances of this class are created by {@link MidiManager#openDevice}.
  *
  * CANDIDATE FOR PUBLIC API
@@ -42,11 +44,10 @@ public final class MidiDevice implements Closeable {
     private Context mContext;
     private ServiceConnection mServiceConnection;
 
+    private final CloseGuard mGuard = CloseGuard.get();
+
     /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server) {
-        mDeviceInfo = deviceInfo;
-        mDeviceServer = server;
-        mContext = null;
-        mServiceConnection = null;
+        this(deviceInfo, server, null, null);
     }
 
     /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server,
@@ -55,6 +56,7 @@ public final class MidiDevice implements Closeable {
         mDeviceServer = server;
         mContext = context;
         mServiceConnection = serviceConnection;
+        mGuard.open("close");
     }
 
     /**
@@ -108,10 +110,23 @@ public final class MidiDevice implements Closeable {
 
     @Override
     public void close() throws IOException {
-        if (mContext != null && mServiceConnection != null) {
-            mContext.unbindService(mServiceConnection);
-            mContext = null;
-            mServiceConnection = null;
+        synchronized (mGuard) {
+            mGuard.close();
+            if (mContext != null && mServiceConnection != null) {
+                mContext.unbindService(mServiceConnection);
+                mContext = null;
+                mServiceConnection = null;
+            }
+        }
+    }
+
+    @Override
+    protected void finalize() throws Throwable {
+        try {
+            mGuard.warnIfOpen();
+            close();
+        } finally {
+            super.finalize();
         }
     }
 
diff --git a/media/java/android/media/midi/MidiDeviceInfo.java b/media/java/android/media/midi/MidiDeviceInfo.java
index b7756fd..1e97258 100644
--- a/media/java/android/media/midi/MidiDeviceInfo.java
+++ b/media/java/android/media/midi/MidiDeviceInfo.java
@@ -31,7 +31,7 @@ import android.os.Parcelable;
  * CANDIDATE FOR PUBLIC API
  * @hide
  */
-public class MidiDeviceInfo implements Parcelable {
+public final class MidiDeviceInfo implements Parcelable {
 
     private static final String TAG = "MidiDeviceInfo";
 
diff --git a/media/java/android/media/midi/MidiDeviceServer.java b/media/java/android/media/midi/MidiDeviceServer.java
index 4d59c63..3b4b6f0 100644
--- a/media/java/android/media/midi/MidiDeviceServer.java
+++ b/media/java/android/media/midi/MidiDeviceServer.java
@@ -24,11 +24,14 @@ import android.os.RemoteException;
 import android.system.OsConstants;
 import android.util.Log;
 
+import dalvik.system.CloseGuard;
+
 import libcore.io.IoUtils;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.HashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * Internal class used for providing an implementation for a MIDI device.
@@ -54,6 +57,12 @@ public final class MidiDeviceServer implements Closeable {
     // MidiOutputPorts for clients connected to our input ports
     private final MidiOutputPort[] mInputPortOutputPorts;
 
+    // List of all MidiInputPorts we created
+    private final CopyOnWriteArrayList<MidiInputPort> mInputPorts
+            = new CopyOnWriteArrayList<MidiInputPort>();
+
+    private final CloseGuard mGuard = CloseGuard.get();
+
     abstract private class PortClient implements IBinder.DeathRecipient {
         final IBinder mToken;
 
@@ -105,6 +114,7 @@ public final class MidiDeviceServer implements Closeable {
         void close() {
             mToken.unlinkToDeath(this, 0);
             mOutputPortDispatchers[mInputPort.getPortNumber()].getSender().disconnect(mInputPort);
+            mInputPorts.remove(mInputPort);
             IoUtils.closeQuietly(mInputPort);
         }
     }
@@ -169,6 +179,7 @@ public final class MidiDeviceServer implements Closeable {
                                                     OsConstants.SOCK_SEQPACKET);
                 MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber);
                 mOutputPortDispatchers[portNumber].getSender().connect(inputPort);
+                mInputPorts.add(inputPort);
                 OutputPortClient client = new OutputPortClient(token, inputPort);
                 synchronized (mPortClients) {
                     mPortClients.put(token, client);
@@ -204,6 +215,8 @@ public final class MidiDeviceServer implements Closeable {
         for (int i = 0; i < numOutputPorts; i++) {
             mOutputPortDispatchers[i] = new MidiDispatcher();
         }
+
+        mGuard.open("close");
     }
 
     /* package */ IMidiDeviceServer getBinderInterface() {
@@ -219,11 +232,35 @@ public final class MidiDeviceServer implements Closeable {
 
     @Override
     public void close() throws IOException {
+        synchronized (mGuard) {
+            mGuard.close();
+
+            for (int i = 0; i < mInputPortCount; i++) {
+                MidiOutputPort outputPort = mInputPortOutputPorts[i];
+                if (outputPort != null) {
+                    IoUtils.closeQuietly(outputPort);
+                    mInputPortOutputPorts[i] = null;
+                }
+            }
+            for (MidiInputPort inputPort : mInputPorts) {
+                IoUtils.closeQuietly(inputPort);
+            }
+            mInputPorts.clear();
+            try {
+                mMidiManager.unregisterDeviceServer(mServer);
+            } catch (RemoteException e) {
+                Log.e(TAG, "RemoteException in unregisterDeviceServer");
+            }
+        }
+    }
+
+    @Override
+    protected void finalize() throws Throwable {
         try {
-            // FIXME - close input and output ports too?
-            mMidiManager.unregisterDeviceServer(mServer);
-        } catch (RemoteException e) {
-            Log.e(TAG, "RemoteException in unregisterDeviceServer");
+            mGuard.warnIfOpen();
+            close();
+        } finally {
+            super.finalize();
         }
     }
 
diff --git a/media/java/android/media/midi/MidiDispatcher.java b/media/java/android/media/midi/MidiDispatcher.java
index b2d8b6f..a5193f1 100644
--- a/media/java/android/media/midi/MidiDispatcher.java
+++ b/media/java/android/media/midi/MidiDispatcher.java
@@ -17,7 +17,7 @@
 package android.media.midi;
 
 import java.io.IOException;
-import java.util.ArrayList;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * Utility class for dispatching MIDI data to a list of {@link MidiReceiver}s.
@@ -29,9 +29,10 @@ import java.util.ArrayList;
  * CANDIDATE FOR PUBLIC API
  * @hide
  */
-public class MidiDispatcher extends MidiReceiver {
+public final class MidiDispatcher extends MidiReceiver {
 
-    private final ArrayList<MidiReceiver> mReceivers = new ArrayList<MidiReceiver>();
+    private final CopyOnWriteArrayList<MidiReceiver> mReceivers
+            = new CopyOnWriteArrayList<MidiReceiver>();
 
     private final MidiSender mSender = new MidiSender() {
         /**
@@ -72,17 +73,12 @@ public class MidiDispatcher extends MidiReceiver {
 
     @Override
     public void receive(byte[] msg, int offset, int count, long timestamp) throws IOException {
-        synchronized (mReceivers) {
-           for (int i = 0; i < mReceivers.size(); ) {
-                MidiReceiver receiver = mReceivers.get(i);
-                try {
-                    receiver.receive(msg, offset, count, timestamp);
-                    i++;    // increment only on success. on failure we remove the receiver
-                            // so i should not be incremented
-                } catch (IOException e) {
-                    // if the receiver fails we remove the receiver but do not propogate the exception
-                    mSender.disconnect(receiver);
-                }
+       for (MidiReceiver receiver : mReceivers) {
+            try {
+                receiver.receive(msg, offset, count, timestamp);
+            } catch (IOException e) {
+                // if the receiver fails we remove the receiver but do not propogate the exception
+                mReceivers.remove(receiver);
             }
         }
     }
diff --git a/media/java/android/media/midi/MidiInputPort.java b/media/java/android/media/midi/MidiInputPort.java
index 5d944cb..8674969 100644
--- a/media/java/android/media/midi/MidiInputPort.java
+++ b/media/java/android/media/midi/MidiInputPort.java
@@ -35,15 +35,16 @@ import java.io.IOException;
  * CANDIDATE FOR PUBLIC API
  * @hide
  */
-public class MidiInputPort extends MidiReceiver implements Closeable {
+public final class MidiInputPort extends MidiReceiver implements Closeable {
     private static final String TAG = "MidiInputPort";
 
-    private final IMidiDeviceServer mDeviceServer;
+    private IMidiDeviceServer mDeviceServer;
     private final IBinder mToken;
     private final int mPortNumber;
     private final FileOutputStream mOutputStream;
 
     private final CloseGuard mGuard = CloseGuard.get();
+    private boolean mIsClosed;
 
     // buffer to use for sending data out our output stream
     private final byte[] mBuffer = new byte[MidiPortImpl.MAX_PACKET_SIZE];
@@ -97,23 +98,27 @@ public class MidiInputPort extends MidiReceiver implements Closeable {
 
     @Override
     public void close() throws IOException {
-        mGuard.close();
-        mOutputStream.close();
-        if (mDeviceServer != null) {
-            try {
-                mDeviceServer.closePort(mToken);
-            } catch (RemoteException e) {
-                Log.e(TAG, "RemoteException in MidiInputPort.close()");
+        synchronized (mGuard) {
+            if (mIsClosed) return;
+            mGuard.close();
+            mOutputStream.close();
+            if (mDeviceServer != null) {
+                try {
+                    mDeviceServer.closePort(mToken);
+                } catch (RemoteException e) {
+                    Log.e(TAG, "RemoteException in MidiInputPort.close()");
+                }
             }
+            mIsClosed = true;
         }
     }
 
     @Override
     protected void finalize() throws Throwable {
         try {
-            if (mGuard != null) {
-                mGuard.warnIfOpen();
-            }
+            mGuard.warnIfOpen();
+            // not safe to make binder calls from finalize()
+            mDeviceServer = null;
             close();
         } finally {
             super.finalize();
diff --git a/media/java/android/media/midi/MidiManager.java b/media/java/android/media/midi/MidiManager.java
index 08ac25a..d7b8c57 100644
--- a/media/java/android/media/midi/MidiManager.java
+++ b/media/java/android/media/midi/MidiManager.java
@@ -42,7 +42,7 @@ import java.util.HashMap;
  * CANDIDATE FOR PUBLIC API
  * @hide
  */
-public class MidiManager {
+public final class MidiManager {
     private static final String TAG = "MidiManager";
 
     private final Context mContext;
diff --git a/media/java/android/media/midi/MidiOutputPort.java b/media/java/android/media/midi/MidiOutputPort.java
index d46b202..b31cdd3 100644
--- a/media/java/android/media/midi/MidiOutputPort.java
+++ b/media/java/android/media/midi/MidiOutputPort.java
@@ -35,16 +35,17 @@ import java.io.IOException;
  * CANDIDATE FOR PUBLIC API
  * @hide
  */
-public class MidiOutputPort extends MidiSender implements Closeable {
+public final class MidiOutputPort extends MidiSender implements Closeable {
     private static final String TAG = "MidiOutputPort";
 
-    private final IMidiDeviceServer mDeviceServer;
+    private IMidiDeviceServer mDeviceServer;
     private final IBinder mToken;
     private final int mPortNumber;
     private final FileInputStream mInputStream;
     private final MidiDispatcher mDispatcher = new MidiDispatcher();
 
     private final CloseGuard mGuard = CloseGuard.get();
+    private boolean mIsClosed;
 
     // This thread reads MIDI events from a socket and distributes them to the list of
     // MidiReceivers attached to this device.
@@ -113,23 +114,28 @@ public class MidiOutputPort extends MidiSender implements Closeable {
 
     @Override
     public void close() throws IOException {
-        mGuard.close();
-        mInputStream.close();
-        if (mDeviceServer != null) {
-            try {
-                mDeviceServer.closePort(mToken);
-            } catch (RemoteException e) {
-                Log.e(TAG, "RemoteException in MidiOutputPort.close()");
+        synchronized (mGuard) {
+            if (mIsClosed) return;
+
+            mGuard.close();
+            mInputStream.close();
+            if (mDeviceServer != null) {
+                try {
+                    mDeviceServer.closePort(mToken);
+                } catch (RemoteException e) {
+                    Log.e(TAG, "RemoteException in MidiOutputPort.close()");
+                }
             }
+            mIsClosed = true;
         }
     }
 
     @Override
     protected void finalize() throws Throwable {
         try {
-            if (mGuard != null) {
-                mGuard.warnIfOpen();
-            }
+            mGuard.warnIfOpen();
+            // not safe to make binder calls from finalize()
+            mDeviceServer = null;
             close();
         } finally {
             super.finalize();