Merge "Properly dispose DeviceManager from AvdManager."

3 files changed, 56 insertions, 29 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java
index 855a7fa..e01bfab 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/launch/EmulatorConfigTab.java
@@ -149,9 +149,17 @@ public class EmulatorConfigTab extends AbstractLaunchConfigurationTab {
     public void createControl(Composite parent) {
         Font font = parent.getFont();
 
-        // reload the AVDs to make sure we are up to date
+        // Reload the AVDs to make sure we are up to date
         try {
-            Sdk.getCurrent().getAvdManager().reloadAvds(NullLogger.getLogger());
+            // SDK can be null if the user opens the dialog before ADT finished
+            // initializing the SDK itself. In this case just don't reload anything
+            // so there's nothing obsolete yet.
+            Sdk sdk = Sdk.getCurrent();
+            if (sdk != null) {
+                AvdManager avdMan = sdk.getAvdManager();
+                assert avdMan != null;
+                avdMan.reloadAvds(NullLogger.getLogger());
+            }
         } catch (AndroidLocationException e1) {
             // this happens if the AVD Manager failed to find the folder in which the AVDs are
             // stored. There isn't much we can do at this point.
diff --git a/sdkmanager/libs/sdklib/src/com/android/sdklib/devices/DeviceManager.java b/sdkmanager/libs/sdklib/src/com/android/sdklib/devices/DeviceManager.java
index 27feb00..d5297d3 100644
--- a/sdkmanager/libs/sdklib/src/com/android/sdklib/devices/DeviceManager.java
+++ b/sdkmanager/libs/sdklib/src/com/android/sdklib/devices/DeviceManager.java
@@ -68,8 +68,8 @@ public class DeviceManager {
     private static List<Device> mUserDevices;
     private static List<Device> mDefaultDevices;
     private static final Object sLock = new Object();
-    private static final List<DevicesChangeListener> listeners =
-        Collections.synchronizedList(new ArrayList<DevicesChangeListener>());
+    private static final List<DevicesChangeListener> sListeners =
+                                        new ArrayList<DevicesChangeListener>();
 
     public static enum DeviceStatus {
         /**
@@ -87,10 +87,8 @@ public class DeviceManager {
     }
 
     // TODO: Refactor this to look more like AvdManager so that we don't have
-    // multiple instances
-    // in the same application, which forces us to parse the XML multiple times
-    // when we don't
-    // to.
+    // multiple instances in the same application, which forces us to parse
+    // the XML multiple times when we don't have to.
     public DeviceManager(ILogger log) {
         mLog = log;
     }
@@ -108,19 +106,29 @@ public class DeviceManager {
 
     /**
      * Register a listener to be notified when the device lists are modified.
-     * @param listener The listener to add
+     *
+     * @param listener The listener to add. Ignored if already registered.
      */
     public void registerListener(DevicesChangeListener listener) {
-        listeners.add(listener);
+        if (listener != null) {
+            synchronized (sListeners) {
+                if (!sListeners.contains(listener)) {
+                    sListeners.add(listener);
+                }
+            }
+        }
     }
 
     /**
      * Removes a listener from the notification list such that it will no longer receive
      * notifications when modifications to the {@link Device} list occur.
+     *
      * @param listener The listener to remove.
      */
     public boolean unregisterListener(DevicesChangeListener listener) {
-        return listeners.remove(listener);
+        synchronized (sListeners) {
+            return sListeners.remove(listener);
+        }
     }
 
     public DeviceStatus getDeviceStatus(
@@ -189,10 +197,10 @@ public class DeviceManager {
     }
 
     /**
-     * Returns all vendor provided {@link Device}s
+     * Returns all vendor-provided {@link Device}s
      *
      * @param sdkLocation Location of the Android SDK
-     * @return A list of vendor provided {@link Device}s
+     * @return A list of vendor-provided {@link Device}s
      */
     public List<Device> getVendorDevices(String sdkLocation) {
         synchronized (sLock) {
@@ -224,9 +232,9 @@ public class DeviceManager {
     }
 
     /**
-     * Returns all user created {@link Device}s
+     * Returns all user-created {@link Device}s
      *
-     * @return All user created {@link Device}s
+     * @return All user-created {@link Device}s
      */
     public List<Device> getUserDevices() {
         synchronized (sLock) {
@@ -244,22 +252,25 @@ public class DeviceManager {
                     mLog.warning("Couldn't load user devices: %1$s", e.getMessage());
                 } catch (SAXException e) {
                     // Probably an old config file which we don't want to overwrite.
-                    // FIXME: userDevicesFile is likely null at this point and below.
-                    String base = userDevicesFile.getAbsoluteFile()+".old";
-                    File renamedConfig = new File(base);
-                    int i = 0;
-                    while (renamedConfig.exists()) {
-                        renamedConfig = new File(base + "." + (i++));
+                    if (userDevicesFile != null) {
+                        String base = userDevicesFile.getAbsoluteFile() + ".old";
+                        File renamedConfig = new File(base);
+                        int i = 0;
+                        while (renamedConfig.exists()) {
+                            renamedConfig = new File(base + '.' + (i++));
+                        }
+                        mLog.error(null, "Error parsing %1$s, backing up to %2$s",
+                                userDevicesFile.getAbsolutePath(), renamedConfig.getAbsolutePath());
+                        userDevicesFile.renameTo(renamedConfig);
                     }
-                    mLog.error(null, "Error parsing %1$s, backing up to %2$s",
-                            userDevicesFile.getAbsolutePath(), renamedConfig.getAbsolutePath());
-                    userDevicesFile.renameTo(renamedConfig);
                 } catch (FileNotFoundException e) {
                     mLog.warning("No user devices found");
                 } catch (ParserConfigurationException e) {
-                    mLog.error(null, "Error parsing %1$s", userDevicesFile.getAbsolutePath());
+                    mLog.error(null, "Error parsing %1$s",
+                            userDevicesFile == null ? "(null)" : userDevicesFile.getAbsolutePath());
                 } catch (IOException e) {
-                    mLog.error(null, "Error parsing %1$s", userDevicesFile.getAbsolutePath());
+                    mLog.error(null, "Error parsing %1$s",
+                            userDevicesFile == null ? "(null)" : userDevicesFile.getAbsolutePath());
                 }
             }
         }
@@ -421,8 +432,8 @@ public class DeviceManager {
     }
 
     private void notifyListeners() {
-        synchronized (listeners) {
-            for (DevicesChangeListener listener : listeners) {
+        synchronized (sListeners) {
+            for (DevicesChangeListener listener : sListeners) {
                 listener.onDevicesChange();
             }
         }
diff --git a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/ui/AvdManagerPage.java b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/ui/AvdManagerPage.java
index c691b77..1074dfa 100755
--- a/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/ui/AvdManagerPage.java
+++ b/sdkmanager/libs/sdkuilib/src/com/android/sdkuilib/internal/repository/ui/AvdManagerPage.java
@@ -25,6 +25,8 @@ import com.android.sdkuilib.internal.widgets.AvdSelector.DisplayMode;
 import com.android.sdkuilib.repository.ISdkChangeListener;
 
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.DisposeEvent;
+import org.eclipse.swt.events.DisposeListener;
 import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Composite;
@@ -39,7 +41,8 @@ import org.eclipse.swt.widgets.Label;
  * thus composed of the {@link AvdManagerWindowImpl1} (the window shell itself) and this
  * page displays the actually list of AVDs and various action buttons.
  */
-public class AvdManagerPage extends Composite implements ISdkChangeListener, DevicesChangeListener {
+public class AvdManagerPage extends Composite
+    implements ISdkChangeListener, DevicesChangeListener, DisposeListener {
 
     private AvdSelector mAvdSelector;
 
@@ -93,6 +96,11 @@ public class AvdManagerPage extends Composite implements ISdkChangeListener, Dev
     }
 
     @Override
+    public void widgetDisposed(DisposeEvent e) {
+        dispose();
+    }
+
+    @Override
     public void dispose() {
         mUpdaterData.removeListener(this);
         mDeviceManager.unregisterListener(this);