MIDI Manager: Add explicit close mechanism for input and output ports

Relying on errors from closing the file descriptor is not reliable
and was resulting in file descriptor leaks in device servers.

Change-Id: Ib5cc22dba493eae6608a12cc6d4178d8390da77b

5 files changed, 172 insertions, 47 deletions

diff --git a/media/java/android/media/midi/IMidiDeviceServer.aidl b/media/java/android/media/midi/IMidiDeviceServer.aidl
index 71914ad..3331aae 100644
--- a/media/java/android/media/midi/IMidiDeviceServer.aidl
+++ b/media/java/android/media/midi/IMidiDeviceServer.aidl
@@ -21,6 +21,7 @@ import android.os.ParcelFileDescriptor;
 /** @hide */
 interface IMidiDeviceServer
 {
-    ParcelFileDescriptor openInputPort(int portNumber);
-    ParcelFileDescriptor openOutputPort(int portNumber);
+    ParcelFileDescriptor openInputPort(IBinder token, int portNumber);
+    ParcelFileDescriptor openOutputPort(IBinder token, int portNumber);
+    void closePort(IBinder token);
 }
diff --git a/media/java/android/media/midi/MidiDevice.java b/media/java/android/media/midi/MidiDevice.java
index a273915..1a39485 100644
--- a/media/java/android/media/midi/MidiDevice.java
+++ b/media/java/android/media/midi/MidiDevice.java
@@ -18,6 +18,8 @@ package android.media.midi;
 
 import android.content.Context;
 import android.content.ServiceConnection;
+import android.os.Binder;
+import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.util.Log;
@@ -36,13 +38,13 @@ public final class MidiDevice implements Closeable {
     private static final String TAG = "MidiDevice";
 
     private final MidiDeviceInfo mDeviceInfo;
-    private final IMidiDeviceServer mServer;
+    private final IMidiDeviceServer mDeviceServer;
     private Context mContext;
     private ServiceConnection mServiceConnection;
 
     /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server) {
         mDeviceInfo = deviceInfo;
-        mServer = server;
+        mDeviceServer = server;
         mContext = null;
         mServiceConnection = null;
     }
@@ -50,7 +52,7 @@ public final class MidiDevice implements Closeable {
     /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server,
             Context context, ServiceConnection serviceConnection) {
         mDeviceInfo = deviceInfo;
-        mServer = server;
+        mDeviceServer = server;
         mContext = context;
         mServiceConnection = serviceConnection;
     }
@@ -72,11 +74,12 @@ public final class MidiDevice implements Closeable {
      */
     public MidiInputPort openInputPort(int portNumber) {
         try {
-            ParcelFileDescriptor pfd = mServer.openInputPort(portNumber);
+            IBinder token = new Binder();
+            ParcelFileDescriptor pfd = mDeviceServer.openInputPort(token, portNumber);
             if (pfd == null) {
                 return null;
             }
-            return new MidiInputPort(pfd, portNumber);
+            return new MidiInputPort(mDeviceServer, token, pfd, portNumber);
         } catch (RemoteException e) {
             Log.e(TAG, "RemoteException in openInputPort");
             return null;
@@ -91,11 +94,12 @@ public final class MidiDevice implements Closeable {
      */
     public MidiOutputPort openOutputPort(int portNumber) {
         try {
-            ParcelFileDescriptor pfd = mServer.openOutputPort(portNumber);
+            IBinder token = new Binder();
+            ParcelFileDescriptor pfd = mDeviceServer.openOutputPort(token, portNumber);
             if (pfd == null) {
                 return null;
             }
-            return new MidiOutputPort(pfd, portNumber);
+            return new MidiOutputPort(mDeviceServer, token, pfd, portNumber);
         } catch (RemoteException e) {
             Log.e(TAG, "RemoteException in openOutputPort");
             return null;
diff --git a/media/java/android/media/midi/MidiDeviceServer.java b/media/java/android/media/midi/MidiDeviceServer.java
index 87fb3c5..4d59c63 100644
--- a/media/java/android/media/midi/MidiDeviceServer.java
+++ b/media/java/android/media/midi/MidiDeviceServer.java
@@ -28,6 +28,7 @@ import libcore.io.IoUtils;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.HashMap;
 
 /**
  * Internal class used for providing an implementation for a MIDI device.
@@ -53,11 +54,68 @@ public final class MidiDeviceServer implements Closeable {
     // MidiOutputPorts for clients connected to our input ports
     private final MidiOutputPort[] mInputPortOutputPorts;
 
+    abstract private class PortClient implements IBinder.DeathRecipient {
+        final IBinder mToken;
+
+        PortClient(IBinder token) {
+            mToken = token;
+
+            try {
+                token.linkToDeath(this, 0);
+            } catch (RemoteException e) {
+                close();
+            }
+        }
+
+        abstract void close();
+
+        @Override
+        public void binderDied() {
+            close();
+        }
+    }
+
+    private class InputPortClient extends PortClient {
+        private final MidiOutputPort mOutputPort;
+
+        InputPortClient(IBinder token, MidiOutputPort outputPort) {
+            super(token);
+            mOutputPort = outputPort;
+        }
+
+        @Override
+        void close() {
+            mToken.unlinkToDeath(this, 0);
+            synchronized (mInputPortOutputPorts) {
+                mInputPortOutputPorts[mOutputPort.getPortNumber()] = null;
+            }
+            IoUtils.closeQuietly(mOutputPort);
+        }
+    }
+
+    private class OutputPortClient extends PortClient {
+        private final MidiInputPort mInputPort;
+
+        OutputPortClient(IBinder token, MidiInputPort inputPort) {
+            super(token);
+            mInputPort = inputPort;
+        }
+
+        @Override
+        void close() {
+            mToken.unlinkToDeath(this, 0);
+            mOutputPortDispatchers[mInputPort.getPortNumber()].getSender().disconnect(mInputPort);
+            IoUtils.closeQuietly(mInputPort);
+        }
+    }
+
+    private final HashMap<IBinder, PortClient> mPortClients = new HashMap<IBinder, PortClient>();
+
     // Binder interface stub for receiving connection requests from clients
     private final IMidiDeviceServer mServer = new IMidiDeviceServer.Stub() {
 
         @Override
-        public ParcelFileDescriptor openInputPort(int portNumber) {
+        public ParcelFileDescriptor openInputPort(IBinder token, int portNumber) {
             if (mDeviceInfo.isPrivate()) {
                 if (Binder.getCallingUid() != Process.myUid()) {
                     throw new SecurityException("Can't access private device from different UID");
@@ -78,25 +136,13 @@ public final class MidiDeviceServer implements Closeable {
                 try {
                     ParcelFileDescriptor[] pair = ParcelFileDescriptor.createSocketPair(
                                                         OsConstants.SOCK_SEQPACKET);
-                    final MidiOutputPort outputPort = new MidiOutputPort(pair[0], portNumber);
+                    MidiOutputPort outputPort = new MidiOutputPort(pair[0], portNumber);
                     mInputPortOutputPorts[portNumber] = outputPort;
-                    final int portNumberF = portNumber;
-                    final MidiReceiver inputPortReceviver = mInputPortReceivers[portNumber];
-
-                    outputPort.connect(new MidiReceiver() {
-                        @Override
-                        public void receive(byte[] msg, int offset, int count, long timestamp)
-                                throws IOException {
-                            try {
-                                inputPortReceviver.receive(msg, offset, count, timestamp);
-                            } catch (IOException e) {
-                                IoUtils.closeQuietly(mInputPortOutputPorts[portNumberF]);
-                                mInputPortOutputPorts[portNumberF] = null;
-                                // FIXME also flush the receiver
-                            }
-                        }
-                    });
-
+                    outputPort.connect(mInputPortReceivers[portNumber]);
+                    InputPortClient client = new InputPortClient(token, outputPort);
+                    synchronized (mPortClients) {
+                        mPortClients.put(token, client);
+                    }
                     return pair[1];
                 } catch (IOException e) {
                     Log.e(TAG, "unable to create ParcelFileDescriptors in openInputPort");
@@ -106,7 +152,7 @@ public final class MidiDeviceServer implements Closeable {
         }
 
         @Override
-        public ParcelFileDescriptor openOutputPort(int portNumber) {
+        public ParcelFileDescriptor openOutputPort(IBinder token, int portNumber) {
             if (mDeviceInfo.isPrivate()) {
                 if (Binder.getCallingUid() != Process.myUid()) {
                     throw new SecurityException("Can't access private device from different UID");
@@ -121,28 +167,28 @@ public final class MidiDeviceServer implements Closeable {
             try {
                 ParcelFileDescriptor[] pair = ParcelFileDescriptor.createSocketPair(
                                                     OsConstants.SOCK_SEQPACKET);
-                final MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber);
-                final MidiSender sender = mOutputPortDispatchers[portNumber].getSender();
-                sender.connect(new MidiReceiver() {
-                        @Override
-                        public void receive(byte[] msg, int offset, int count, long timestamp)
-                                throws IOException {
-                            try {
-                                inputPort.receive(msg, offset, count, timestamp);
-                            } catch (IOException e) {
-                                IoUtils.closeQuietly(inputPort);
-                                sender.disconnect(this);
-                                // FIXME also flush the receiver?
-                            }
-                        }
-                    });
-
+                MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber);
+                mOutputPortDispatchers[portNumber].getSender().connect(inputPort);
+                OutputPortClient client = new OutputPortClient(token, inputPort);
+                synchronized (mPortClients) {
+                    mPortClients.put(token, client);
+                }
                 return pair[1];
             } catch (IOException e) {
                 Log.e(TAG, "unable to create ParcelFileDescriptors in openOutputPort");
                 return null;
             }
         }
+
+        @Override
+        public void closePort(IBinder token) {
+            synchronized (mPortClients) {
+                PortClient client = mPortClients.remove(token);
+                if (client != null) {
+                    client.close();
+                }
+            }
+        }
     };
 
     /* package */ MidiDeviceServer(IMidiManager midiManager, MidiReceiver[] inputPortReceivers,
diff --git a/media/java/android/media/midi/MidiInputPort.java b/media/java/android/media/midi/MidiInputPort.java
index fb70e89..5d944cb 100644
--- a/media/java/android/media/midi/MidiInputPort.java
+++ b/media/java/android/media/midi/MidiInputPort.java
@@ -16,7 +16,12 @@
 
 package android.media.midi;
 
+import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
+import android.os.RemoteException;
+import android.util.Log;
+
+import dalvik.system.CloseGuard;
 
 import libcore.io.IoUtils;
 
@@ -31,16 +36,29 @@ import java.io.IOException;
  * @hide
  */
 public class MidiInputPort extends MidiReceiver implements Closeable {
+    private static final String TAG = "MidiInputPort";
 
+    private final IMidiDeviceServer mDeviceServer;
+    private final IBinder mToken;
     private final int mPortNumber;
     private final FileOutputStream mOutputStream;
 
+    private final CloseGuard mGuard = CloseGuard.get();
+
     // buffer to use for sending data out our output stream
     private final byte[] mBuffer = new byte[MidiPortImpl.MAX_PACKET_SIZE];
 
-  /* package */ MidiInputPort(ParcelFileDescriptor pfd, int portNumber) {
+    /* package */ MidiInputPort(IMidiDeviceServer server, IBinder token,
+            ParcelFileDescriptor pfd, int portNumber) {
+        mDeviceServer = server;
+        mToken = token;
         mPortNumber = portNumber;
         mOutputStream = new ParcelFileDescriptor.AutoCloseOutputStream(pfd);
+        mGuard.open("close");
+    }
+
+    /* package */ MidiInputPort(ParcelFileDescriptor pfd, int portNumber) {
+        this(null, null, pfd, portNumber);
     }
 
     /**
@@ -79,6 +97,26 @@ 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()");
+            }
+        }
+    }
+
+    @Override
+    protected void finalize() throws Throwable {
+        try {
+            if (mGuard != null) {
+                mGuard.warnIfOpen();
+            }
+            close();
+        } finally {
+            super.finalize();
+        }
     }
 }
diff --git a/media/java/android/media/midi/MidiOutputPort.java b/media/java/android/media/midi/MidiOutputPort.java
index 8b99e90..d46b202 100644
--- a/media/java/android/media/midi/MidiOutputPort.java
+++ b/media/java/android/media/midi/MidiOutputPort.java
@@ -16,9 +16,13 @@
 
 package android.media.midi;
 
+import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
+import android.os.RemoteException;
 import android.util.Log;
 
+import dalvik.system.CloseGuard;
+
 import libcore.io.IoUtils;
 
 import java.io.Closeable;
@@ -34,10 +38,14 @@ import java.io.IOException;
 public class MidiOutputPort extends MidiSender implements Closeable {
     private static final String TAG = "MidiOutputPort";
 
+    private final 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();
+
     // This thread reads MIDI events from a socket and distributes them to the list of
     // MidiReceivers attached to this device.
     private final Thread mThread = new Thread() {
@@ -70,10 +78,18 @@ public class MidiOutputPort extends MidiSender implements Closeable {
         }
     };
 
-    /* package */ MidiOutputPort(ParcelFileDescriptor pfd, int portNumber) {
+    /* package */ MidiOutputPort(IMidiDeviceServer server, IBinder token,
+            ParcelFileDescriptor pfd, int portNumber) {
+        mDeviceServer = server;
+        mToken = token;
         mPortNumber = portNumber;
         mInputStream = new ParcelFileDescriptor.AutoCloseInputStream(pfd);
         mThread.start();
+        mGuard.open("close");
+    }
+
+    /* package */ MidiOutputPort(ParcelFileDescriptor pfd, int portNumber) {
+        this(null, null, pfd, portNumber);
     }
 
     /**
@@ -97,6 +113,26 @@ 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()");
+            }
+        }
+    }
+
+    @Override
+    protected void finalize() throws Throwable {
+        try {
+            if (mGuard != null) {
+                mGuard.warnIfOpen();
+            }
+            close();
+        } finally {
+            super.finalize();
+        }
     }
 }