From f57c3a857c7b812586637d1148ae17686ee15837 Mon Sep 17 00:00:00 2001 From: Devrandom Date: Sun, 24 Aug 2014 14:15:50 -0700 Subject: [PATCH] Orchid: better thread tracking also, fix race condition in closing circuits on shutdown --- .../orchid/circuits/CircuitCreationTask.java | 11 ++++++- .../orchid/circuits/CircuitManagerImpl.java | 32 +++++++++++++++++-- .../orchid/circuits/guards/EntryGuards.java | 12 ++++++- .../subgraph/orchid/dashboard/Dashboard.java | 11 ++++++- .../orchid/directory/DescriptorCache.java | 12 ++++++- .../downloader/DirectoryDownloadTask.java | 11 ++++++- 6 files changed, 82 insertions(+), 7 deletions(-) diff --git a/orchid/src/com/subgraph/orchid/circuits/CircuitCreationTask.java b/orchid/src/com/subgraph/orchid/circuits/CircuitCreationTask.java index e8ba0276..17a759ad 100644 --- a/orchid/src/com/subgraph/orchid/circuits/CircuitCreationTask.java +++ b/orchid/src/com/subgraph/orchid/circuits/CircuitCreationTask.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; @@ -52,7 +53,15 @@ public class CircuitCreationTask implements Runnable { this.circuitManager = circuitManager; this.initializationTracker = initializationTracker; this.pathChooser = pathChooser; - this.executor = Executors.newCachedThreadPool(); + this.executor = Executors.newCachedThreadPool(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName("CircuitCreationTask worker"); + t.setDaemon(true); + return t; + } + }); this.buildHandler = createCircuitBuildHandler(); this.internalBuildHandler = createInternalCircuitBuildHandler(); this.predictor = new CircuitPredictor(); diff --git a/orchid/src/com/subgraph/orchid/circuits/CircuitManagerImpl.java b/orchid/src/com/subgraph/orchid/circuits/CircuitManagerImpl.java index 12b3ee79..b2617897 100644 --- a/orchid/src/com/subgraph/orchid/circuits/CircuitManagerImpl.java +++ b/orchid/src/com/subgraph/orchid/circuits/CircuitManagerImpl.java @@ -11,6 +11,7 @@ import java.util.Queue; import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.locks.ReentrantLock; @@ -59,13 +60,23 @@ public class CircuitManagerImpl implements CircuitManager, DashboardRenderable { private int pendingInternalCircuitCount = 0; private final TorRandom random; private final PendingExitStreams pendingExitStreams; - private final ScheduledExecutorService scheduledExecutor = Executors.newSingleThreadScheduledExecutor(); + private final ScheduledExecutorService scheduledExecutor = Executors.newSingleThreadScheduledExecutor(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName("CircuitManager worker"); + t.setDaemon(true); + return t; + } + }); private final CircuitCreationTask circuitCreationTask; private final TorInitializationTracker initializationTracker; private final CircuitPathChooser pathChooser; private final HiddenServiceManager hiddenServiceManager; private final ReentrantLock lock = Threading.lock("circuitManager"); + private boolean isBuilding = false; + public CircuitManagerImpl(TorConfig config, DirectoryDownloaderImpl directoryDownloader, Directory directory, ConnectionCache connectionCache, TorInitializationTracker initializationTracker) { this.config = config; this.directory = directory; @@ -87,13 +98,20 @@ public class CircuitManagerImpl implements CircuitManager, DashboardRenderable { } public void startBuildingCircuits() { - scheduledExecutor.scheduleAtFixedRate(circuitCreationTask, 0, 1000, TimeUnit.MILLISECONDS); + lock.lock(); + try { + isBuilding = true; + scheduledExecutor.scheduleAtFixedRate(circuitCreationTask, 0, 1000, TimeUnit.MILLISECONDS); + } finally { + lock.unlock(); + } } public void stopBuildingCircuits(boolean killCircuits) { lock.lock(); try { + isBuilding = false; scheduledExecutor.shutdownNow(); if (killCircuits) { List circuits = new ArrayList(activeCircuits); @@ -111,6 +129,16 @@ public class CircuitManagerImpl implements CircuitManager, DashboardRenderable { } void addActiveCircuit(CircuitImpl circuit) { + lock.lock(); + + try { + if (!isBuilding) { + circuit.destroyCircuit(); + } + } finally { + lock.unlock(); + } + synchronized (activeCircuits) { activeCircuits.add(circuit); activeCircuits.notifyAll(); diff --git a/orchid/src/com/subgraph/orchid/circuits/guards/EntryGuards.java b/orchid/src/com/subgraph/orchid/circuits/guards/EntryGuards.java index 958470a4..05a6d3a4 100644 --- a/orchid/src/com/subgraph/orchid/circuits/guards/EntryGuards.java +++ b/orchid/src/com/subgraph/orchid/circuits/guards/EntryGuards.java @@ -7,6 +7,8 @@ import java.util.List; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; @@ -47,7 +49,15 @@ public class EntryGuards { this.pendingProbes = new HashSet(); this.bridges = new Bridges(config, directoryDownloader); this.lock = new Object(); - this.executor = Executors.newCachedThreadPool(); + this.executor = Executors.newCachedThreadPool(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName("EntryGuards worker"); + t.setDaemon(true); + return t; + } + }); } public boolean isUsingBridges() { diff --git a/orchid/src/com/subgraph/orchid/dashboard/Dashboard.java b/orchid/src/com/subgraph/orchid/dashboard/Dashboard.java index 966e920f..9974a49f 100644 --- a/orchid/src/com/subgraph/orchid/dashboard/Dashboard.java +++ b/orchid/src/com/subgraph/orchid/dashboard/Dashboard.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import java.util.logging.Logger; import com.subgraph.orchid.data.IPv4Address; @@ -37,7 +38,15 @@ public class Dashboard implements DashboardRenderable, DashboardRenderer { public Dashboard() { renderables = new CopyOnWriteArrayList(); renderables.add(this); - executor = Executors.newCachedThreadPool(); + executor = Executors.newCachedThreadPool(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName("Dashboard worker"); + t.setDaemon(true); + return t; + } + }); listeningPort = chooseListeningPort(); } diff --git a/orchid/src/com/subgraph/orchid/directory/DescriptorCache.java b/orchid/src/com/subgraph/orchid/directory/DescriptorCache.java index e40acef9..c3109c3d 100644 --- a/orchid/src/com/subgraph/orchid/directory/DescriptorCache.java +++ b/orchid/src/com/subgraph/orchid/directory/DescriptorCache.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; @@ -23,7 +24,16 @@ public abstract class DescriptorCache { private final DescriptorCacheData data; private final DirectoryStore store; - private final ScheduledExecutorService rebuildExecutor = Executors.newScheduledThreadPool(1); + private final ScheduledExecutorService rebuildExecutor = Executors.newScheduledThreadPool(1, new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName("DescriptorCache rebuild worker"); + t.setDaemon(true); + return t; + } + }); + private final CacheFile cacheFile; private final CacheFile journalFile; diff --git a/orchid/src/com/subgraph/orchid/directory/downloader/DirectoryDownloadTask.java b/orchid/src/com/subgraph/orchid/directory/downloader/DirectoryDownloadTask.java index 761e6264..526bec8b 100644 --- a/orchid/src/com/subgraph/orchid/directory/downloader/DirectoryDownloadTask.java +++ b/orchid/src/com/subgraph/orchid/directory/downloader/DirectoryDownloadTask.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; @@ -31,7 +32,15 @@ public class DirectoryDownloadTask implements Runnable { private final TorRandom random; private final DescriptorProcessor descriptorProcessor; - private final ExecutorService executor = Executors.newCachedThreadPool(); + private final ExecutorService executor = Executors.newCachedThreadPool(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setName("DirectoryDownloadTask worker"); + t.setDaemon(true); + return t; + } + }); private volatile boolean isDownloadingCertificates; private volatile boolean isDownloadingConsensus;