From 99e3b5f33b5aa84e8b417f7341192b5202dbf62e Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 16 Nov 2014 17:20:04 -0800 Subject: [PATCH] Rewrite a large portion of the computer manager service to fix some thread leaks and improve performance --- .../computers/ComputerManagerService.java | 109 +++++++++++------- 1 file changed, 70 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/com/limelight/computers/ComputerManagerService.java b/app/src/main/java/com/limelight/computers/ComputerManagerService.java index 227a9f67..20662c00 100644 --- a/app/src/main/java/com/limelight/computers/ComputerManagerService.java +++ b/app/src/main/java/com/limelight/computers/ComputerManagerService.java @@ -1,8 +1,7 @@ package com.limelight.computers; import java.net.InetAddress; -import java.util.HashMap; -import java.util.List; +import java.util.LinkedList; import java.util.concurrent.atomic.AtomicInteger; import com.limelight.LimeLog; @@ -30,7 +29,7 @@ public class ComputerManagerService extends Service { private AtomicInteger dbRefCount = new AtomicInteger(0); private IdentityManager idManager; - private final HashMap pollingThreads = new HashMap(); + private final LinkedList pollingTuples = new LinkedList(); private ComputerManagerListener listener = null; private AtomicInteger activePolls = new AtomicInteger(0); @@ -102,20 +101,8 @@ public class ComputerManagerService extends Service { @Override public void run() { while (!isInterrupted()) { - ComputerDetails originalDetails = new ComputerDetails(); - originalDetails.update(details); - // Check if this poll has modified the details - if (runPoll(details) && !originalDetails.equals(details)) { - // Replace our thread entry with the new one - synchronized (pollingThreads) { - if (pollingThreads.remove(originalDetails) != null) { - // This could have gone away in the meantime, so don't - // add it back if it has - pollingThreads.put(details, this); - } - } - } + runPoll(details); // Wait until the next polling interval try { @@ -137,24 +124,16 @@ public class ComputerManagerService extends Service { // Start mDNS autodiscovery too discoveryBinder.startDiscovery(MDNS_QUERY_PERIOD_MS); - - // Start polling known machines - if (!getLocalDatabaseReference()) { - return; - } - List computerList = dbManager.getAllComputers(); - releaseLocalDatabaseReference(); - synchronized (pollingThreads) { - for (ComputerDetails computer : computerList) { + synchronized (pollingTuples) { + for (PollingTuple tuple : pollingTuples) { // This polling thread might already be there - if (!pollingThreads.containsKey(computer)) { + if (tuple.thread == null) { // Report this computer initially - listener.notifyComputerUpdated(computer); + listener.notifyComputerUpdated(tuple.computer); - Thread t = createPollingThread(computer); - pollingThreads.put(computer, t); - t.start(); + tuple.thread = createPollingThread(tuple.computer); + tuple.thread.start(); } } } @@ -208,11 +187,14 @@ public class ComputerManagerService extends Service { discoveryBinder.stopDiscovery(); // Stop polling - synchronized (pollingThreads) { - for (Thread t : pollingThreads.values()) { - t.interrupt(); + synchronized (pollingTuples) { + for (PollingTuple tuple : pollingTuples) { + if (tuple.thread != null) { + // Interrupt and remove the thread + tuple.thread.interrupt(); + tuple.thread = null; + } } - pollingThreads.clear(); } // Remove the listener @@ -249,13 +231,26 @@ public class ComputerManagerService extends Service { fakeDetails.remoteIp = addr; // Spawn a thread for this computer - synchronized (pollingThreads) { + synchronized (pollingTuples) { // This polling thread might already be there - if (!pollingThreads.containsKey(fakeDetails)) { - Thread t = createPollingThread(fakeDetails); - pollingThreads.put(fakeDetails, t); - t.start(); + for (PollingTuple tuple : pollingTuples) { + if (tuple.computer.localIp.equals(addr) || + tuple.computer.remoteIp.equals(addr)) { + // This is the same computer + if (tuple.thread == null) { + tuple.thread = createPollingThread(fakeDetails); + tuple.thread.start(); + } + + // Found an entry so we're done + return; + } } + + // If we got here, we didn't find an entry + PollingTuple tuple = new PollingTuple(fakeDetails, createPollingThread(fakeDetails)); + pollingTuples.add(tuple); + tuple.thread.start(); } } @@ -279,6 +274,20 @@ public class ComputerManagerService extends Service { // Remove it from the database dbManager.deleteComputer(name); + + synchronized (pollingTuples) { + // Remove the computer from the computer list + for (PollingTuple tuple : pollingTuples) { + if (tuple.computer.name.equals(name)) { + if (tuple.thread != null) { + // Interrupt the thread on this entry + tuple.thread.interrupt(); + } + pollingTuples.remove(tuple); + break; + } + } + } releaseLocalDatabaseReference(); } @@ -376,6 +385,18 @@ public class ComputerManagerService extends Service { // Initialize the DB dbManager = new ComputerDatabaseManager(this); dbRefCount.set(1); + + // Grab known machines into our computer list + if (!getLocalDatabaseReference()) { + return; + } + + for (ComputerDetails computer : dbManager.getAllComputers()) { + // Add this computer without a thread + pollingTuples.add(new PollingTuple(computer, null)); + } + + releaseLocalDatabaseReference(); } @Override @@ -396,3 +417,13 @@ public class ComputerManagerService extends Service { return binder; } } + +class PollingTuple { + public Thread thread; + public ComputerDetails computer; + + public PollingTuple(ComputerDetails computer, Thread thread) { + this.computer = computer; + this.thread = thread; + } +} \ No newline at end of file