aboutsummaryrefslogtreecommitdiffstats
path: root/ddms/libs
diff options
context:
space:
mode:
authorSiva Velusamy <vsiva@google.com>2012-02-06 12:18:12 -0800
committerSiva Velusamy <vsiva@google.com>2012-02-06 14:38:32 -0800
commit29928a2b799230c6bc222152823cec0b6887695e (patch)
tree004984f1219873b1e2e50d1fa2e5ff46a2a15c10 /ddms/libs
parent3c16d164a654ac15466c7476388f6b5fa2519798 (diff)
downloadsdk-29928a2b799230c6bc222152823cec0b6887695e.zip
sdk-29928a2b799230c6bc222152823cec0b6887695e.tar.gz
sdk-29928a2b799230c6bc222152823cec0b6887695e.tar.bz2
logcat: Fix deadlock
LogCatReceiverFactory has a deadlock scenario that occurs like this: 1. When a device is disconnected, removeReceiverFor() is called from a DDMS device monitor thread. This thread holds certain DDMS internal locks. removeReceiverFor() is synchronized on LogCatReceiverFactory. So we have: DDMS thread - holds device lock removeReceiverFor - waiting to obtain lock for "this" object. 2. At the same time, a separate thread could call newReceiver. As part of creating a new receiver, we add a device change listener. So we have: newReceiver - holds "this" object. create device change listener - waiting for device lock. As a result, the system deadlocks. This patch fixes this condition by calling the removeReceiverFor from a non DDMS thread since we don't really care about DDMS locks here. Change-Id: Ic4ef4589acdcd7980864da23cde760debc4cfbb8
Diffstat (limited to 'ddms/libs')
-rw-r--r--ddms/libs/ddmuilib/src/com/android/ddmuilib/logcat/LogCatReceiverFactory.java32
1 files changed, 29 insertions, 3 deletions
diff --git a/ddms/libs/ddmuilib/src/com/android/ddmuilib/logcat/LogCatReceiverFactory.java b/ddms/libs/ddmuilib/src/com/android/ddmuilib/logcat/LogCatReceiverFactory.java
index 5617988..5b25e17 100644
--- a/ddms/libs/ddmuilib/src/com/android/ddmuilib/logcat/LogCatReceiverFactory.java
+++ b/ddms/libs/ddmuilib/src/com/android/ddmuilib/logcat/LogCatReceiverFactory.java
@@ -38,8 +38,20 @@ public class LogCatReceiverFactory {
private LogCatReceiverFactory() {
AndroidDebugBridge.addDeviceChangeListener(new IDeviceChangeListener() {
@Override
- public void deviceDisconnected(IDevice device) {
- removeReceiverFor(device);
+ public void deviceDisconnected(final IDevice device) {
+ // The deviceDisconnected() is called from DDMS code that holds
+ // multiple locks regarding list of clients, etc.
+ // It so happens that #newReceiver() below adds a clientChangeListener
+ // which requires those locks as well. So if we call
+ // #removeReceiverFor from a DDMS/Monitor thread, we could end up
+ // in a deadlock. As a result, we spawn a separate thread that
+ // doesn't hold any of the DDMS locks to remove the receiver.
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ removeReceiverFor(device); }
+ }, "Remove logcat receiver for " + device.getSerialNumber());
+ t.start();
}
@Override
@@ -52,8 +64,22 @@ public class LogCatReceiverFactory {
});
}
+ /**
+ * Remove existing logcat receivers. This method should not be called from a DDMS thread
+ * context that might be holding locks. Doing so could result in a deadlock with the following
+ * two threads locked up: <ul>
+ * <li> {@link #removeReceiverFor(IDevice)} waiting to lock {@link LogCatReceiverFactory},
+ * while holding a DDMS monitor internal lock. </li>
+ * <li> {@link #newReceiver(IDevice, IPreferenceStore)} holding {@link LogCatReceiverFactory}
+ * while attempting to obtain a DDMS monitor lock. </li>
+ * </ul>
+ */
private synchronized void removeReceiverFor(IDevice device) {
- mReceiverCache.remove(device.getSerialNumber());
+ LogCatReceiver r = mReceiverCache.get(device.getSerialNumber());
+ if (r != null) {
+ r.stop();
+ mReceiverCache.remove(device.getSerialNumber());
+ }
}
public synchronized LogCatReceiver newReceiver(IDevice device, IPreferenceStore prefs) {