summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Glenn Kasten <gkasten@google.com> 2016-08-17 08:45:39 -0700
committer Glenn Kasten <gkasten@google.com> 2016-08-17 08:45:39 -0700
commit1fda533936415b52d537b0056304ff5bd8af8519 (patch)
tree0fe7a682841772dbe94148d8b4b6036a39b46288
parentc5b376ab66d20defca0a60a4ba74bdbb31444d55 (diff)
Squashed commit of the following:
commit efa6f355b06675aa4d0879fd279e22c16d5c046c Author: Mikhail Naganov <mnaganov@google.com> Date: Wed Aug 10 12:25:13 2016 -0700 MIDI: Use server-side socket in blocking mode for virtual devices Since virtual MIDI servers may misbehave, blocking mode will throttle them if clients are not coping with their sending speed. Bug: 29413812 Change-Id: I9c4a2a7a7ea3ea060c93fedc7d0f033427c557c9 commit 755dfb5f83749d3963c63d98d692307f8271c804 Author: Mikhail Naganov <mnaganov@google.com> Date: Fri Jul 8 13:26:19 2016 -0700 Protect MIDI framework against client blocks in MidiReceiver.onSend Make the server-side socket non-blocking when creating MidiOutputPort for clients. Thus if a client ceases to read from its side of the socket pair, the server will just fail to write instead of blocking. One drawback is that the MidiOutputPort on the client can't indicate that it has become dysfunctional, but it's not possible without changing the API. Bug: 29413812 Change-Id: I9dfcbdd214a815cea8fd1365324fd78ca459268a commit c740b13953761f58233ac651a0b5227733b1bdcc Author: Mikhail Naganov <mnaganov@google.com> Date: Fri Jun 17 04:11:25 2016 -0700 UsbMidiDevice: Clean up terminology and fix comments When working with physical MIDI devices, an *input* stream is used for reading from *output* port of the device, and vice versa. Thus, using "input" and "output" without specifying whether it's a stream or a port is confusing. Clarify names of counter variables, and fix a couple of comments that were incorrect due to this confusion. No functional changes. Change-Id: If561eaca4bade94e9296d2c703c9fcebc91296e2 commit 4269c6417287737624f6165a8bbeb5aa427de9a0 Author: Glenn Kasten <gkasten@google.com> Date: Thu May 5 18:49:16 2016 -0700 Update MIDI package summary Bug: 28625060 Change-Id: If552ca8e1a0666d402b5f536699bf3fb09c1e324 commit 862d40b73168bde7d0be5280d997985c18061014 Author: Phil Burk <philburk@google.com> Date: Tue Apr 19 15:56:24 2016 -0700 MidiDevice: do not open ports on closed device Fix involves client side mIsDeviceClosed flag. Bug: 24949216 Change-Id: I666284a787fbb9a710d2372fb424e8e54f6a2825 Signed-off-by: Phil Burk <philburk@google.com> commit 6f1de358b9f2616e03f4655f01454770915ddd66 Author: Phil Burk <philburk@google.com> Date: Mon Apr 18 16:05:28 2016 -0700 MidiService: fix resource leak The proxy object was being used to match when adding or removing objects. But they are different each time. So now we use an asBinder() object. Bug: 28153736 Change-Id: I1bccebf1e9464668db757ff08b41902d0cf0e3a7 Signed-off-by: Phil Burk <philburk@google.com> commit f7386bd535bb8a1d7f8df8f44a1748ab770c991a Author: Phil Burk <philburk@google.com> Date: Tue Apr 5 14:19:53 2016 -0700 MidiDevice: fix connectPorts for same Process If connectPorts() was called for a device in the same process then the connection would die when the ParcelFileDescriptor was closed. Bug: 26406775 Change-Id: Id0538452593b4761ac2a93d366ade76d2e35ce73 Signed-off-by: Phil Burk <philburk@google.com> Change-Id: I4dfc2a2cbaf04bf1a790ae2cb39bf74fb5bb16ac
-rw-r--r--core/java/com/android/internal/midi/MidiDispatcher.java41
-rw-r--r--media/java/android/media/midi/IMidiDeviceServer.aidl3
-rw-r--r--media/java/android/media/midi/MidiDevice.java39
-rw-r--r--media/java/android/media/midi/MidiDeviceServer.java53
-rw-r--r--media/java/android/media/midi/MidiOutputPort.java2
-rw-r--r--media/java/android/media/midi/package.html9
-rw-r--r--services/midi/java/com/android/server/midi/MidiService.java16
-rw-r--r--services/usb/java/com/android/server/usb/UsbMidiDevice.java30
8 files changed, 148 insertions, 45 deletions
diff --git a/core/java/com/android/internal/midi/MidiDispatcher.java b/core/java/com/android/internal/midi/MidiDispatcher.java
index 1a3c37c79cd2..c16628a9c692 100644
--- a/core/java/com/android/internal/midi/MidiDispatcher.java
+++ b/core/java/com/android/internal/midi/MidiDispatcher.java
@@ -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) {
- receiver.flush();
+ 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);
+ }
+ }
}
}
}
diff --git a/media/java/android/media/midi/IMidiDeviceServer.aidl b/media/java/android/media/midi/IMidiDeviceServer.aidl
index c2cc2b9f9379..d5115de4a048 100644
--- a/media/java/android/media/midi/IMidiDeviceServer.aidl
+++ b/media/java/android/media/midi/IMidiDeviceServer.aidl
@@ -28,7 +28,8 @@ interface IMidiDeviceServer
void closeDevice();
// connects the input port pfd to the specified output port
- void connectPorts(IBinder token, in ParcelFileDescriptor pfd, int outputPortNumber);
+ // Returns the PID of the called process.
+ int connectPorts(IBinder token, in ParcelFileDescriptor pfd, int outputPortNumber);
MidiDeviceInfo getDeviceInfo();
void setDeviceInfo(in MidiDeviceInfo deviceInfo);
diff --git a/media/java/android/media/midi/MidiDevice.java b/media/java/android/media/midi/MidiDevice.java
index e1990cd6fbbb..da44ca682945 100644
--- a/media/java/android/media/midi/MidiDevice.java
+++ b/media/java/android/media/midi/MidiDevice.java
@@ -19,6 +19,7 @@ package android.media.midi;
import android.os.Binder;
import android.os.IBinder;
import android.os.ParcelFileDescriptor;
+import android.os.Process;
import android.os.RemoteException;
import android.util.Log;
@@ -41,6 +42,7 @@ public final class MidiDevice implements Closeable {
private final IMidiManager mMidiManager;
private final IBinder mClientToken;
private final IBinder mDeviceToken;
+ private boolean mIsDeviceClosed;
private final CloseGuard mGuard = CloseGuard.get();
@@ -122,6 +124,9 @@ public final class MidiDevice implements Closeable {
* or null in case of failure.
*/
public MidiInputPort openInputPort(int portNumber) {
+ if (mIsDeviceClosed) {
+ return null;
+ }
try {
IBinder token = new Binder();
ParcelFileDescriptor pfd = mDeviceServer.openInputPort(token, portNumber);
@@ -145,6 +150,9 @@ public final class MidiDevice implements Closeable {
* or null in case of failure.
*/
public MidiOutputPort openOutputPort(int portNumber) {
+ if (mIsDeviceClosed) {
+ return null;
+ }
try {
IBinder token = new Binder();
ParcelFileDescriptor pfd = mDeviceServer.openOutputPort(token, portNumber);
@@ -174,16 +182,26 @@ public final class MidiDevice implements Closeable {
if (outputPortNumber < 0 || outputPortNumber >= mDeviceInfo.getOutputPortCount()) {
throw new IllegalArgumentException("outputPortNumber out of range");
}
+ if (mIsDeviceClosed) {
+ return null;
+ }
ParcelFileDescriptor pfd = inputPort.claimFileDescriptor();
if (pfd == null) {
return null;
}
- try {
+ try {
IBinder token = new Binder();
- mDeviceServer.connectPorts(token, pfd, outputPortNumber);
- // close our copy of the file descriptor
- IoUtils.closeQuietly(pfd);
+ int calleePid = mDeviceServer.connectPorts(token, pfd, outputPortNumber);
+ // If the service is a different Process then it will duplicate the pfd
+ // and we can safely close this one.
+ // But if the service is in the same Process then closing the pfd will
+ // kill the connection. So don't do that.
+ if (calleePid != Process.myPid()) {
+ // close our copy of the file descriptor
+ IoUtils.closeQuietly(pfd);
+ }
+
return new MidiConnection(token, inputPort);
} catch (RemoteException e) {
Log.e(TAG, "RemoteException in connectPorts");
@@ -194,11 +212,14 @@ public final class MidiDevice implements Closeable {
@Override
public void close() throws IOException {
synchronized (mGuard) {
- mGuard.close();
- try {
- mMidiManager.closeDevice(mClientToken, mDeviceToken);
- } catch (RemoteException e) {
- Log.e(TAG, "RemoteException in closeDevice");
+ if (!mIsDeviceClosed) {
+ mGuard.close();
+ mIsDeviceClosed = true;
+ try {
+ mMidiManager.closeDevice(mClientToken, mDeviceToken);
+ } catch (RemoteException e) {
+ Log.e(TAG, "RemoteException in closeDevice");
+ }
}
}
}
diff --git a/media/java/android/media/midi/MidiDeviceServer.java b/media/java/android/media/midi/MidiDeviceServer.java
index 19ff62460155..4c49f677c031 100644
--- a/media/java/android/media/midi/MidiDeviceServer.java
+++ b/media/java/android/media/midi/MidiDeviceServer.java
@@ -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,12 @@ public final class MidiDeviceServer implements Closeable {
ParcelFileDescriptor[] pair = ParcelFileDescriptor.createSocketPair(
OsConstants.SOCK_SEQPACKET);
MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber);
+ // Undo the default blocking-mode of the server-side socket for
+ // physical devices to avoid stalling the Java device handler if
+ // client app code gets stuck inside 'onSend' handler.
+ if (mDeviceInfo.getType() != MidiDeviceInfo.TYPE_VIRTUAL) {
+ IoUtils.setBlocking(pair[0].getFileDescriptor(), false);
+ }
MidiDispatcher dispatcher = mOutputPortDispatchers[portNumber];
synchronized (dispatcher) {
dispatcher.getSender().connect(inputPort);
@@ -228,6 +245,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 +257,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
@@ -254,7 +281,7 @@ public final class MidiDeviceServer implements Closeable {
}
@Override
- public void connectPorts(IBinder token, ParcelFileDescriptor pfd,
+ public int connectPorts(IBinder token, ParcelFileDescriptor pfd,
int outputPortNumber) {
MidiInputPort inputPort = new MidiInputPort(pfd, outputPortNumber);
MidiDispatcher dispatcher = mOutputPortDispatchers[outputPortNumber];
@@ -270,6 +297,10 @@ 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
}
@Override
@@ -302,7 +333,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];
@@ -311,6 +342,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) {
diff --git a/media/java/android/media/midi/MidiOutputPort.java b/media/java/android/media/midi/MidiOutputPort.java
index 0096995bac28..54c31e34ef16 100644
--- a/media/java/android/media/midi/MidiOutputPort.java
+++ b/media/java/android/media/midi/MidiOutputPort.java
@@ -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);
}
diff --git a/media/java/android/media/midi/package.html b/media/java/android/media/midi/package.html
index 45fb5791dfa7..eead8d8cd16b 100644
--- a/media/java/android/media/midi/package.html
+++ b/media/java/android/media/midi/package.html
@@ -1,11 +1,12 @@
<html>
<body>
-<p>Android MIDI User Guide</p>
-
-<h1 id=overview>Overview</h1>
+<p>
+Provides classes for sending and receiving messages using the standard MIDI
+event protocol over USB, Bluetooth LE, and virtual (inter-app) transports.
+</p>
-<p>This document describes how to use the Android MIDI API in Java.</p>
+<h1 id=overview>Overview</h1>
<p>The Android MIDI package allows users to:</p>
diff --git a/services/midi/java/com/android/server/midi/MidiService.java b/services/midi/java/com/android/server/midi/MidiService.java
index c6d5a7e268b7..723be2481d6f 100644
--- a/services/midi/java/com/android/server/midi/MidiService.java
+++ b/services/midi/java/com/android/server/midi/MidiService.java
@@ -126,8 +126,8 @@ public class MidiService extends IMidiManager.Stub {
// This client's PID
private final int mPid;
// List of all receivers for this client
- private final ArrayList<IMidiDeviceListener> mListeners
- = new ArrayList<IMidiDeviceListener>();
+ private final HashMap<IBinder, IMidiDeviceListener> mListeners
+ = new HashMap<IBinder, IMidiDeviceListener>();
// List of all device connections for this client
private final HashMap<IBinder, DeviceConnection> mDeviceConnections
= new HashMap<IBinder, DeviceConnection>();
@@ -143,11 +143,13 @@ public class MidiService extends IMidiManager.Stub {
}
public void addListener(IMidiDeviceListener listener) {
- mListeners.add(listener);
+ // Use asBinder() so that we can match it in removeListener().
+ // The listener proxy objects themselves do not match.
+ mListeners.put(listener.asBinder(), listener);
}
public void removeListener(IMidiDeviceListener listener) {
- mListeners.remove(listener);
+ mListeners.remove(listener.asBinder());
if (mListeners.size() == 0 && mDeviceConnections.size() == 0) {
close();
}
@@ -184,7 +186,7 @@ public class MidiService extends IMidiManager.Stub {
MidiDeviceInfo deviceInfo = device.getDeviceInfo();
try {
- for (IMidiDeviceListener listener : mListeners) {
+ for (IMidiDeviceListener listener : mListeners.values()) {
listener.onDeviceAdded(deviceInfo);
}
} catch (RemoteException e) {
@@ -198,7 +200,7 @@ public class MidiService extends IMidiManager.Stub {
MidiDeviceInfo deviceInfo = device.getDeviceInfo();
try {
- for (IMidiDeviceListener listener : mListeners) {
+ for (IMidiDeviceListener listener : mListeners.values()) {
listener.onDeviceRemoved(deviceInfo);
}
} catch (RemoteException e) {
@@ -211,7 +213,7 @@ public class MidiService extends IMidiManager.Stub {
if (!device.isUidAllowed(mUid)) return;
try {
- for (IMidiDeviceListener listener : mListeners) {
+ for (IMidiDeviceListener listener : mListeners.values()) {
listener.onDeviceStatusChanged(status);
}
} catch (RemoteException e) {
diff --git a/services/usb/java/com/android/server/usb/UsbMidiDevice.java b/services/usb/java/com/android/server/usb/UsbMidiDevice.java
index 46ce7a0a1aaf..cd19795747f5 100644
--- a/services/usb/java/com/android/server/usb/UsbMidiDevice.java
+++ b/services/usb/java/com/android/server/usb/UsbMidiDevice.java
@@ -51,7 +51,7 @@ public final class UsbMidiDevice implements Closeable {
private MidiDeviceServer mServer;
- // event schedulers for each output port
+ // event schedulers for each input port of the physical device
private MidiEventScheduler[] mEventSchedulers;
private static final int BUFFER_SIZE = 512;
@@ -160,9 +160,9 @@ public final class UsbMidiDevice implements Closeable {
mSubdeviceCount = subdeviceCount;
// FIXME - support devices with different number of input and output ports
- int inputCount = subdeviceCount;
- mInputPortReceivers = new InputReceiverProxy[inputCount];
- for (int port = 0; port < inputCount; port++) {
+ int inputPortCount = subdeviceCount;
+ mInputPortReceivers = new InputReceiverProxy[inputPortCount];
+ for (int port = 0; port < inputPortCount; port++) {
mInputPortReceivers[port] = new InputReceiverProxy();
}
}
@@ -176,14 +176,14 @@ public final class UsbMidiDevice implements Closeable {
}
mFileDescriptors = fileDescriptors;
- int inputCount = fileDescriptors.length;
+ int inputStreamCount = fileDescriptors.length;
// last file descriptor returned from nativeOpen() is only used for unblocking Os.poll()
// in our input thread
- int outputCount = fileDescriptors.length - 1;
+ int outputStreamCount = fileDescriptors.length - 1;
- mPollFDs = new StructPollfd[inputCount];
- mInputStreams = new FileInputStream[inputCount];
- for (int i = 0; i < inputCount; i++) {
+ mPollFDs = new StructPollfd[inputStreamCount];
+ mInputStreams = new FileInputStream[inputStreamCount];
+ for (int i = 0; i < inputStreamCount; i++) {
FileDescriptor fd = fileDescriptors[i];
StructPollfd pollfd = new StructPollfd();
pollfd.fd = fd;
@@ -192,9 +192,9 @@ public final class UsbMidiDevice implements Closeable {
mInputStreams[i] = new FileInputStream(fd);
}
- mOutputStreams = new FileOutputStream[outputCount];
- mEventSchedulers = new MidiEventScheduler[outputCount];
- for (int i = 0; i < outputCount; i++) {
+ mOutputStreams = new FileOutputStream[outputStreamCount];
+ mEventSchedulers = new MidiEventScheduler[outputStreamCount];
+ for (int i = 0; i < outputStreamCount; i++) {
mOutputStreams[i] = new FileOutputStream(fileDescriptors[i]);
MidiEventScheduler scheduler = new MidiEventScheduler();
@@ -204,7 +204,7 @@ public final class UsbMidiDevice implements Closeable {
final MidiReceiver[] outputReceivers = mServer.getOutputPortReceivers();
- // Create input thread which will read from all input ports
+ // Create input thread which will read from all output ports of the physical device
new Thread("UsbMidiDevice input thread") {
@Override
public void run() {
@@ -249,8 +249,8 @@ public final class UsbMidiDevice implements Closeable {
}
}.start();
- // Create output thread for each output port
- for (int port = 0; port < outputCount; port++) {
+ // Create output thread for each input port of the physical device
+ for (int port = 0; port < outputStreamCount; port++) {
final MidiEventScheduler eventSchedulerF = mEventSchedulers[port];
final FileOutputStream outputStreamF = mOutputStreams[port];
final int portF = port;