Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4a3d7ed4 authored by Mike Lockwood's avatar Mike Lockwood
Browse files

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
parent eebc98ff

media/java/android/media/midi/IMidiDeviceServer.aidl

+3 −2
Original line number Diff line number Diff line
@@ -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);

}

media/java/android/media/midi/MidiDevice.java

+11 −7
Original line number Diff line number Diff line
@@ -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;

media/java/android/media/midi/MidiDeviceServer.java

+82 −36
Original line number Diff line number Diff line
@@ -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,

media/java/android/media/midi/MidiInputPort.java

+39 −1
Original line number Diff line number Diff line
@@ -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();

        }

    }

}

media/java/android/media/midi/MidiOutputPort.java

+37 −1
Original line number Diff line number Diff line
@@ -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();

        }

    }

}