Merge "Protect MIDI framework against client blocks in MidiReceiver.onSend" (232ac757) · Commits · e / os / android_frameworks_base

core/java/com/android/internal/midi/MidiDispatcher.java

+37 −4

Original line number	Diff line number	Diff line
		@@ -26,11 +26,23 @@ import java.util.concurrent.CopyOnWriteArrayList;
		* Utility class for dispatching MIDI data to a list of {@link android.media.midi.MidiReceiver}s.
		* This class subclasses {@link android.media.midi.MidiReceiver} and dispatches any data it receives
		* to its receiver list. Any receivers that throw an exception upon receiving data will
		* be automatically removed from the receiver list, but no IOException will be returned
		* from the dispatcher's {@link android.media.midi.MidiReceiver#onSend} in that case.
		* be automatically removed from the receiver list. If a MidiReceiverFailureHandler has been
		* provided to the MidiDispatcher, it will be notified about the failure, but the exception
		* itself will be swallowed.
		*/
		public final class MidiDispatcher extends MidiReceiver {

		// MidiDispatcher's client and MidiReceiver's owner can be different
		// classes (e.g. MidiDeviceService is a client, but MidiDeviceServer is
		// the owner), and errors occuring during sending need to be reported
		// to the owner rather than to the sender.
		//
		// Note that the callbacks will be called on the sender's thread.
		public interface MidiReceiverFailureHandler {
		void onReceiverFailure(MidiReceiver receiver, IOException failure);
		}

		private final MidiReceiverFailureHandler mFailureHandler;
		private final CopyOnWriteArrayList<MidiReceiver> mReceivers
		= new CopyOnWriteArrayList<MidiReceiver>();

		@@ -46,6 +58,14 @@ public final class MidiDispatcher extends MidiReceiver {
		}
		};

		public MidiDispatcher() {
		this(null);
		}

		public MidiDispatcher(MidiReceiverFailureHandler failureHandler) {
		mFailureHandler = failureHandler;
		}

		/**
		* Returns the number of {@link android.media.midi.MidiReceiver}s this dispatcher contains.
		* @return the number of receivers
		@@ -70,8 +90,13 @@ public final class MidiDispatcher extends MidiReceiver {
		try {
		receiver.send(msg, offset, count, timestamp);
		} catch (IOException e) {
		// if the receiver fails we remove the receiver but do not propagate the exception
		// If the receiver fails we remove the receiver but do not propagate the exception.
		// Note that this may also happen if the client code stalls, and thus underlying
		// MidiInputPort.onSend has raised IOException for EAGAIN / EWOULDBLOCK error.
		mReceivers.remove(receiver);
		if (mFailureHandler != null) {
		mFailureHandler.onReceiverFailure(receiver, e);
		}
		}
		}
		}
		@@ -79,7 +104,15 @@ public final class MidiDispatcher extends MidiReceiver {
		@Override
		public void onFlush() throws IOException {
		for (MidiReceiver receiver : mReceivers) {
		try {
		receiver.flush();
		} catch (IOException e) {
		// This is just a special case of 'send' thus handle in the same way.
		mReceivers.remove(receiver);
		if (mFailureHandler != null) {
		mFailureHandler.onReceiverFailure(receiver, e);
		}
		}
		}
		}
		}

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

+45 −3

Original line number	Diff line number	Diff line
		@@ -73,6 +73,10 @@ public final class MidiDeviceServer implements Closeable {

		private final Callback mCallback;

		private final HashMap<IBinder, PortClient> mPortClients = new HashMap<IBinder, PortClient>();
		private final HashMap<MidiInputPort, PortClient> mInputPortClients =
		new HashMap<MidiInputPort, PortClient>();

		public interface Callback {
		/**
		* Called to notify when an our device status has changed
		@@ -102,6 +106,10 @@ public final class MidiDeviceServer implements Closeable {

		abstract void close();

		MidiInputPort getInputPort() {
		return null;
		}

		@Override
		public void binderDied() {
		close();
		@@ -152,9 +160,12 @@ public final class MidiDeviceServer implements Closeable {
		mInputPorts.remove(mInputPort);
		IoUtils.closeQuietly(mInputPort);
		}
		}

		private final HashMap<IBinder, PortClient> mPortClients = new HashMap<IBinder, PortClient>();
		@Override
		MidiInputPort getInputPort() {
		return mInputPort;
		}
		}

		// Binder interface stub for receiving connection requests from clients
		private final IMidiDeviceServer mServer = new IMidiDeviceServer.Stub() {
		@@ -215,6 +226,10 @@ public final class MidiDeviceServer implements Closeable {
		ParcelFileDescriptor[] pair = ParcelFileDescriptor.createSocketPair(
		OsConstants.SOCK_SEQPACKET);
		MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber);
		// Configure the server-side socket in non-blocking mode to avoid stalling
		// the entire MIDI framework if client app code gets stuck inside 'onSend'
		// handler.
		IoUtils.setBlocking(pair[0].getFileDescriptor(), false);
		MidiDispatcher dispatcher = mOutputPortDispatchers[portNumber];
		synchronized (dispatcher) {
		dispatcher.getSender().connect(inputPort);
		@@ -228,6 +243,9 @@ public final class MidiDeviceServer implements Closeable {
		synchronized (mPortClients) {
		mPortClients.put(token, client);
		}
		synchronized (mInputPortClients) {
		mInputPortClients.put(inputPort, client);
		}
		return pair[1];
		} catch (IOException e) {
		Log.e(TAG, "unable to create ParcelFileDescriptors in openOutputPort");
		@@ -237,12 +255,19 @@ public final class MidiDeviceServer implements Closeable {

		@Override
		public void closePort(IBinder token) {
		MidiInputPort inputPort = null;
		synchronized (mPortClients) {
		PortClient client = mPortClients.remove(token);
		if (client != null) {
		inputPort = client.getInputPort();
		client.close();
		}
		}
		if (inputPort != null) {
		synchronized (mInputPortClients) {
		mInputPortClients.remove(inputPort);
		}
		}
		}

		@Override
		@@ -270,6 +295,9 @@ public final class MidiDeviceServer implements Closeable {
		synchronized (mPortClients) {
		mPortClients.put(token, client);
		}
		synchronized (mInputPortClients) {
		mInputPortClients.put(inputPort, client);
		}
		return Process.myPid(); // for caller to detect same process ID
		}

		@@ -303,7 +331,7 @@ public final class MidiDeviceServer implements Closeable {

		mOutputPortDispatchers = new MidiDispatcher[numOutputPorts];
		for (int i = 0; i < numOutputPorts; i++) {
		mOutputPortDispatchers[i] = new MidiDispatcher();
		mOutputPortDispatchers[i] = new MidiDispatcher(mInputPortFailureHandler);
		}

		mInputPortOpen = new boolean[mInputPortCount];
		@@ -312,6 +340,20 @@ public final class MidiDeviceServer implements Closeable {
		mGuard.open("close");
		}

		private final MidiDispatcher.MidiReceiverFailureHandler mInputPortFailureHandler =
		new MidiDispatcher.MidiReceiverFailureHandler() {
		public void onReceiverFailure(MidiReceiver receiver, IOException failure) {
		Log.e(TAG, "MidiInputPort failed to send data", failure);
		PortClient client = null;
		synchronized (mInputPortClients) {
		client = mInputPortClients.remove(receiver);
		}
		if (client != null) {
		client.close();
		}
		}
		};

		// Constructor for MidiDeviceService.onCreate()
		/* package */ MidiDeviceServer(IMidiManager midiManager, MidiReceiver[] inputPortReceivers,
		MidiDeviceInfo deviceInfo, Callback callback) {

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

+1 −1

Original line number	Diff line number	Diff line
		@@ -83,7 +83,7 @@ public final class MidiOutputPort extends MidiSender implements Closeable {
		}
		} catch (IOException e) {
		// FIXME report I/O failure?
		Log.e(TAG, "read failed");
		Log.e(TAG, "read failed", e);
		} finally {
		IoUtils.closeQuietly(mInputStream);
		}