From d43e85d918522e931c1f1733226e1372a73e8894 Mon Sep 17 00:00:00 2001 From: Arthur Little <1690572+littleaj@users.noreply.github.com> Date: Wed, 7 Mar 2018 14:56:22 -0800 Subject: [PATCH] Fix thread leak (#568) * named all our threads. added thread name to loggers. registered CdsProfileFetcher to be shutdown by sdk mechanism. upgraded to Servlet 3.0.1 spec. Added listener to trigger shutdown during undeploy. added undeploy.sh so tests can be written for this scenario. * fixed compile error; added close() * explicitly called a log file (ls wasn't picking the correct one). increased the number of lines to tail from 25 to 50. added option to specify a different log file. added usage on error from tail command. * fixed log messages from undeploy.sh (copy paste error) fixed command * updated docker file to include the package with 'ps' command. change 'ADD' directive to use wildcard. * added startServer and stopServer scripts for debugging * reverted Thread.sleep changes; superfluous * added release note * refactored ThreadFactory creation to be a bit more readable. --- CHANGELOG.md | 1 + .../internal/logger/InternalAgentLogger.java | 3 +- .../docker/DockerContextInitializer.java | 5 +- .../docker/internal/DockerContextPoller.java | 4 +- .../ActiveTransmissionFileSystemOutput.java | 15 ++--- .../common/ActiveTransmissionLoader.java | 3 +- .../ActiveTransmissionNetworkOutput.java | 16 ++---- .../channel/common/ApacheSender42.java | 12 +++- .../common/TransmissionPolicyManager.java | 14 ++--- .../channel/common/TransmitterImpl.java | 17 +++--- .../sampling/AdaptiveTelemetrySampler.java | 10 +--- .../internal/logger/InternalLogger.java | 3 +- .../PerformanceCounterContainer.java | 10 +--- .../internal/quickpulse/QuickPulse.java | 4 +- .../shutdown/SDKShutdownActivity.java | 57 ++++++++++++++----- .../internal/util/ThreadPoolUtils.java | 29 +++++++++- web/build.gradle | 2 +- ...icationInsightsServletContextListener.java | 24 ++++++++ .../internal/WebRequestTrackingFilter.java | 2 + .../correlation/AppProfileFetcher.java | 4 +- .../correlation/CdsProfileFetcher.java | 23 ++++++-- .../mocks/MockHttpAsyncClientWrapper.java | 8 +-- .../correlation/mocks/MockProfileFetcher.java | 6 ++ 23 files changed, 176 insertions(+), 96 deletions(-) create mode 100644 web/src/main/java/com/microsoft/applicationinsights/web/internal/ApplicationInsightsServletContextListener.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a65429939b..482949852b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Fixed performance issue on SDK startup. - Fixed PageView telemetry data not being reported. - Fixed Issue #526 (NPE in MapUtil.copy()) +- Fixed Issue #513 (Memory leak in SDKShutdownActivity). This fix upgrades our Servlet version from 2.5 to 3.0. The SDK must now be run on an application server supporting Servlet 3.0. ## Version 2.0.0-BETA - Updating various dependencies to latest version diff --git a/agent/src/main/java/com/microsoft/applicationinsights/agent/internal/logger/InternalAgentLogger.java b/agent/src/main/java/com/microsoft/applicationinsights/agent/internal/logger/InternalAgentLogger.java index 6e3f2bbbc1a..37a1b0fbdb5 100644 --- a/agent/src/main/java/com/microsoft/applicationinsights/agent/internal/logger/InternalAgentLogger.java +++ b/agent/src/main/java/com/microsoft/applicationinsights/agent/internal/logger/InternalAgentLogger.java @@ -161,7 +161,8 @@ private static String createMessage(String prefix, String message, Object... arg currentDateAsString = dateFormatter.format(new Date()); } String formattedMessage = String.format(message, args); - String theMessage = String.format("%s %s, %d: %s", prefix, currentDateAsString, Thread.currentThread().getId(), formattedMessage); + final Thread thisThread = Thread.currentThread(); + String theMessage = String.format("%s %s, %d(%s): %s", prefix, currentDateAsString, thisThread.getId(), thisThread.getName(), formattedMessage); return theMessage; } diff --git a/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/DockerContextInitializer.java b/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/DockerContextInitializer.java index 3d3a1c199d4..0cfaef210f7 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/DockerContextInitializer.java +++ b/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/DockerContextInitializer.java @@ -23,7 +23,10 @@ import com.microsoft.applicationinsights.TelemetryConfiguration; import com.microsoft.applicationinsights.extensibility.TelemetryInitializer; -import com.microsoft.applicationinsights.extensibility.initializer.docker.internal.*; +import com.microsoft.applicationinsights.extensibility.initializer.docker.internal.Constants; +import com.microsoft.applicationinsights.extensibility.initializer.docker.internal.DockerContext; +import com.microsoft.applicationinsights.extensibility.initializer.docker.internal.DockerContextPoller; +import com.microsoft.applicationinsights.extensibility.initializer.docker.internal.FileFactory; import com.microsoft.applicationinsights.internal.logger.InternalLogger; import com.microsoft.applicationinsights.internal.util.LocalStringsUtils; import com.microsoft.applicationinsights.telemetry.Telemetry; diff --git a/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/internal/DockerContextPoller.java b/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/internal/DockerContextPoller.java index 30042d6df18..a5c9fff4a68 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/internal/DockerContextPoller.java +++ b/core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/internal/DockerContextPoller.java @@ -41,9 +41,9 @@ protected DockerContextPoller(File contextFile, DockerContextFactory dockerConte } public DockerContextPoller(String contextFileDirectory) { + this(new File(contextFileDirectory + "/" + CONTEXT_FILE_NAME), new DockerContextFactory()); this.setDaemon(true); - this.contextFile = new File(contextFileDirectory + "/" + CONTEXT_FILE_NAME); - this.dockerContextFactory = new DockerContextFactory(); + this.setName(DockerContextPoller.class.getSimpleName()); } @Override diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionFileSystemOutput.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionFileSystemOutput.java index f39f32c155c..25a73953b83 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionFileSystemOutput.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionFileSystemOutput.java @@ -22,9 +22,9 @@ package com.microsoft.applicationinsights.internal.channel.common; import java.util.concurrent.RejectedExecutionException; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import com.google.common.base.Preconditions; import com.microsoft.applicationinsights.internal.channel.TransmissionOutput; @@ -38,11 +38,11 @@ * Created by gupele on 12/22/2014. */ public final class ActiveTransmissionFileSystemOutput implements TransmissionOutput { + private static final AtomicInteger INSTANCE_ID_POOL = new AtomicInteger(1); private final ThreadPoolExecutor threadPool; - private final TransmissionOutput actualOutput; - private final TransmissionPolicyStateFetcher transmissionPolicy; + private final int instanceId = INSTANCE_ID_POOL.getAndIncrement(); public ActiveTransmissionFileSystemOutput(TransmissionOutput actualOutput, TransmissionPolicyStateFetcher transmissionPolicy) { Preconditions.checkNotNull(transmissionPolicy, "transmissionPolicy must be a non-null value"); @@ -52,14 +52,7 @@ public ActiveTransmissionFileSystemOutput(TransmissionOutput actualOutput, Trans this.transmissionPolicy = transmissionPolicy; threadPool = ThreadPoolUtils.newLimitedThreadPool(1, 3, 20L, 1024); - threadPool.setThreadFactory(new ThreadFactory() { - @Override - public Thread newThread(Runnable r) { - Thread thread = new Thread(r); - thread.setDaemon(true); - return thread; - } - }); + threadPool.setThreadFactory(ThreadPoolUtils.createDaemonThreadFactory(ActiveTransmissionFileSystemOutput.class, instanceId)); } @Override diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionLoader.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionLoader.java index a78f19a3b67..f957657e3ff 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionLoader.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionLoader.java @@ -88,6 +88,7 @@ public ActiveTransmissionLoader(final TransmissionFileSystemOutput fileSystem, this.fileSystem = fileSystem; this.dispatcher = dispatcher; threads = new Thread[numberOfThreads]; + final String threadNameFmt = String.format("%s-worker-%%d", ActiveTransmissionLoader.class.getSimpleName()); for (int i = 0; i < numberOfThreads; ++i) { threads[i] = new Thread(new Runnable() { @Override @@ -130,7 +131,7 @@ public void run() { // TODO: check whether we need to pause after exception } } - }); + }, String.format(threadNameFmt, i)); threads[i].setDaemon(true); }} diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionNetworkOutput.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionNetworkOutput.java index 6043d2f84fa..ffd2b342bc7 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionNetworkOutput.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionNetworkOutput.java @@ -22,9 +22,9 @@ package com.microsoft.applicationinsights.internal.channel.common; import java.util.concurrent.RejectedExecutionException; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import com.google.common.base.Preconditions; import com.microsoft.applicationinsights.internal.channel.TransmissionOutput; @@ -38,14 +38,13 @@ public final class ActiveTransmissionNetworkOutput implements TransmissionOutput private final static int DEFAULT_MIN_NUMBER_OF_THREADS = 7; private final static int DEFAULT_MAX_NUMBER_OF_THREADS = 7; private final static long DEFAULT_REMOVE_IDLE_THREAD_TIMEOUT_IN_SECONDS = 60L; + private final static AtomicInteger INTSTANCE_ID_POOL = new AtomicInteger(1); private final int maxThreads; - private final ThreadPoolExecutor outputThreads; - private final TransmissionOutput actualOutput; - private final TransmissionPolicyStateFetcher transmissionPolicy; + private final int instanceId = INTSTANCE_ID_POOL.getAndIncrement(); public ActiveTransmissionNetworkOutput(TransmissionOutput actualOutput, TransmissionPolicyStateFetcher transmissionPolicy) { this(actualOutput, transmissionPolicy, DEFAULT_MAX_MESSAGES_IN_BUFFER); @@ -63,14 +62,7 @@ public ActiveTransmissionNetworkOutput(TransmissionOutput actualOutput, Transmis maxThreads, DEFAULT_REMOVE_IDLE_THREAD_TIMEOUT_IN_SECONDS, maxMessagesInBuffer); - outputThreads.setThreadFactory(new ThreadFactory() { - @Override - public Thread newThread(Runnable r) { - Thread thread = new Thread(r); - thread.setDaemon(true); - return thread; - } - }); + outputThreads.setThreadFactory(ThreadPoolUtils.createDaemonThreadFactory(ActiveTransmissionNetworkOutput.class, instanceId)); } @Override diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ApacheSender42.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ApacheSender42.java index 2cd5c1fc974..e9e608e1ec7 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ApacheSender42.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ApacheSender42.java @@ -37,13 +37,14 @@ */ final class ApacheSender42 implements ApacheSender { - private HttpClient httpClient; + private final PoolingClientConnectionManager cm; + private final HttpClient httpClient; public ApacheSender42() { - PoolingClientConnectionManager cm = new PoolingClientConnectionManager(); + cm = new PoolingClientConnectionManager(); cm.setMaxTotal(DEFAULT_MAX_TOTAL_CONNECTIONS); cm.setDefaultMaxPerRoute(DEFAULT_MAX_CONNECTIONS_PER_ROUTE); - + httpClient = new DefaultHttpClient(cm); HttpParams params = httpClient.getParams(); @@ -65,6 +66,11 @@ public void dispose(HttpResponse response) { @Override public void close() { + try { + cm.shutdown(); + } catch (Exception e) { + // chomp + } } @Override diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionPolicyManager.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionPolicyManager.java index 4583a3b1604..ee95f4823a9 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionPolicyManager.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionPolicyManager.java @@ -24,8 +24,8 @@ import java.util.Calendar; import java.util.Date; import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import com.microsoft.applicationinsights.internal.logger.InternalLogger; @@ -47,6 +47,7 @@ * Created by gupele on 6/29/2015. */ public final class TransmissionPolicyManager implements Stoppable { + private static final AtomicInteger INSTANCE_ID_POOL = new AtomicInteger(1); // The future date the the transmission is blocked private Date suspensionDate; @@ -61,6 +62,8 @@ public final class TransmissionPolicyManager implements Stoppable { private final TransmissionPolicyState policyState = new TransmissionPolicyState(); private boolean throttlingIsEnabled = true; + private final int instanceId = INSTANCE_ID_POOL.getAndIncrement(); + /** * The class will be activated when a timeout expires */ @@ -149,14 +152,7 @@ private synchronized void createScheduler() { } threads = new ScheduledThreadPoolExecutor(1); - threads.setThreadFactory(new ThreadFactory() { - @Override - public Thread newThread(Runnable r) { - Thread thread = new Thread(r); - thread.setDaemon(true); - return thread; - } - }); + threads.setThreadFactory(ThreadPoolUtils.createDaemonThreadFactory(TransmissionPolicyManager.class, instanceId)); SDKShutdownActivity.INSTANCE.register(this); } diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmitterImpl.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmitterImpl.java index 67ae7a4d5f6..41c71a88d01 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmitterImpl.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmitterImpl.java @@ -24,8 +24,8 @@ import java.util.Collection; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.Semaphore; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import com.microsoft.applicationinsights.internal.channel.TelemetriesTransmitter; import com.microsoft.applicationinsights.internal.channel.TelemetrySerializer; @@ -112,7 +112,9 @@ public void run() { } } - private final static int MAX_PENDING_SCHEDULE_REQUESTS = 16384; + private static final int MAX_PENDING_SCHEDULE_REQUESTS = 16384; + + private static final AtomicInteger INSTANCE_ID_POOL = new AtomicInteger(1); private final TransmissionDispatcher transmissionDispatcher; @@ -124,6 +126,8 @@ public void run() { private final Semaphore semaphore; + private final int instanceId = INSTANCE_ID_POOL.getAndIncrement(); + public TransmitterImpl(TransmissionDispatcher transmissionDispatcher, TelemetrySerializer serializer, TransmissionsLoader transmissionsLoader) { Preconditions.checkNotNull(transmissionDispatcher, "transmissionDispatcher must be non-null value"); Preconditions.checkNotNull(serializer, "serializer must be non-null value"); @@ -135,14 +139,7 @@ public TransmitterImpl(TransmissionDispatcher transmissionDispatcher, TelemetryS semaphore = new Semaphore(MAX_PENDING_SCHEDULE_REQUESTS); threadPool = new ScheduledThreadPoolExecutor(2); - threadPool.setThreadFactory(new ThreadFactory() { - @Override - public Thread newThread(Runnable r) { - Thread thread = new Thread(r); - thread.setDaemon(true); - return thread; - } - }); + threadPool.setThreadFactory(ThreadPoolUtils.createDaemonThreadFactory(TransmitterImpl.class, instanceId)); this.transmissionsLoader = transmissionsLoader; this.transmissionsLoader.load(false); diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/sampling/AdaptiveTelemetrySampler.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/sampling/AdaptiveTelemetrySampler.java index d8db0c9e086..30e50ec16a9 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/sampling/AdaptiveTelemetrySampler.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/sampling/AdaptiveTelemetrySampler.java @@ -24,7 +24,6 @@ import java.util.Date; import java.util.Set; import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -232,14 +231,7 @@ public boolean isSampledIn(Telemetry telemetry) { private void createTimerThread() { threads = new ScheduledThreadPoolExecutor(1); - threads.setThreadFactory(new ThreadFactory() { - @Override - public Thread newThread(Runnable r) { - Thread thread = new Thread(r); - thread.setDaemon(true); - return thread; - } - }); + threads.setThreadFactory(ThreadPoolUtils.createDaemonThreadFactory(AdaptiveTelemetrySampler.class)); } private int getIntValueOrDefault(String name, String valueAsString, int defaultValue, int minValue, int maxValue) { diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/logger/InternalLogger.java b/core/src/main/java/com/microsoft/applicationinsights/internal/logger/InternalLogger.java index 7d1aebacef5..9e816e37c25 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/logger/InternalLogger.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/logger/InternalLogger.java @@ -226,7 +226,8 @@ private static String createMessage(String prefix, String message, Object... arg currentDateAsString = dateFormatter.format(new Date()); } String formattedMessage = String.format(message, args); - String theMessage = String.format("%s %s, %d: %s", prefix, currentDateAsString, Thread.currentThread().getId(), formattedMessage); + final Thread thisThread = Thread.currentThread(); + String theMessage = String.format("%s %s, %d(%s): %s", prefix, currentDateAsString, thisThread.getId(), thisThread.getName(), formattedMessage); return theMessage; } diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/PerformanceCounterContainer.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/PerformanceCounterContainer.java index b2eca53fac3..6536e6256d2 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/PerformanceCounterContainer.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/PerformanceCounterContainer.java @@ -25,7 +25,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.ThreadFactory; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -259,14 +258,7 @@ public void run() { private void createThreadToCollect() { threads = new ScheduledThreadPoolExecutor(1); - threads.setThreadFactory(new ThreadFactory() { - @Override - public Thread newThread(Runnable r) { - Thread thread = new Thread(r); - thread.setDaemon(true); - return thread; - } - }); + threads.setThreadFactory(ThreadPoolUtils.createDaemonThreadFactory(PerformanceCounterContainer.class)); } public void setPlugin(PerformanceCountersCollectionPlugin plugin) { diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/quickpulse/QuickPulse.java b/core/src/main/java/com/microsoft/applicationinsights/internal/quickpulse/QuickPulse.java index ce3b7071eeb..a083bea0db2 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/quickpulse/QuickPulse.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/quickpulse/QuickPulse.java @@ -80,11 +80,11 @@ public void initialize() { coordinator = new DefaultQuickPulseCoordinator(coordinatorInitData); - senderThread = new Thread(quickPulseDataSender); + senderThread = new Thread(quickPulseDataSender, QuickPulseDataSender.class.getSimpleName()); senderThread.setDaemon(true); senderThread.start(); - thread = new Thread(coordinator); + thread = new Thread(coordinator, DefaultQuickPulseCoordinator.class.getSimpleName()); thread.setDaemon(true); thread.start(); diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/shutdown/SDKShutdownActivity.java b/core/src/main/java/com/microsoft/applicationinsights/internal/shutdown/SDKShutdownActivity.java index 80434ddf0cf..da61d3b2c0c 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/shutdown/SDKShutdownActivity.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/shutdown/SDKShutdownActivity.java @@ -21,6 +21,8 @@ package com.microsoft.applicationinsights.internal.shutdown; +import java.io.Closeable; +import java.util.List; import java.util.ArrayList; import java.util.concurrent.TimeUnit; @@ -44,11 +46,12 @@ public enum SDKShutdownActivity { * 1. The class should not throw an exception * 2. The class 'run' method should exit as soon as possible */ - private static class SDKShutdownThread extends Thread { + private static class SDKShutdownAction implements Runnable { private boolean stopped = false; - private final ArrayList fetchers = new ArrayList(); - private final ArrayList stoppables = new ArrayList(); + private final List fetchers = new ArrayList(); + private final List stoppables = new ArrayList(); + private final List closeables = new ArrayList(); public synchronized void register(ChannelFetcher fetcher) { fetchers.add(fetcher); @@ -58,7 +61,8 @@ public synchronized void register(Stoppable stoppable) { stoppables.add(stoppable); } - public SDKShutdownThread() { + public synchronized void register(Closeable closeable) { + closeables.add(closeable); } @Override @@ -71,6 +75,7 @@ public synchronized void run() { try { stopChannels(); stopStoppables(); + closeClosables(); } finally { // As the last step, the SDK gracefully closes the Internal Logger stopInternalLogger(); @@ -118,25 +123,51 @@ private void stopStoppables() { } } } + private void closeClosables() { + for (Closeable c : closeables) { + try { + c.close(); + } catch (ThreadDeath td) { + throw td; + } catch (Throwable t) { + try { + InternalLogger.INSTANCE.error("Failed to close closeable class '%s': %s", c.getClass().getName(), t.toString()); + InternalLogger.INSTANCE.trace("Stack trace: %s", ExceptionUtils.getStackTrace(t)); + } catch (ThreadDeath td2) { + throw td2; + } catch (Throwable t2) { + // chomp + } + } + } + } } - private static volatile SDKShutdownThread shutdownThread; + private static volatile SDKShutdownAction shutdownAction; public void register(ChannelFetcher fetcher) { - getShutdownThread().register(fetcher); + getShutdownAction().register(fetcher); } public void register(Stoppable stoppable) { - getShutdownThread().register(stoppable); + getShutdownAction().register(stoppable); + } + + public void register(Closeable closable) { + getShutdownAction().register(closable); + } + + public void stopAll() { + getShutdownAction().run(); } - private SDKShutdownThread getShutdownThread() { - if (shutdownThread == null) { + private SDKShutdownAction getShutdownAction() { + if (shutdownAction == null) { synchronized (this) { - if (shutdownThread == null) { + if (shutdownAction == null) { try { - shutdownThread = new SDKShutdownThread(); - Runtime.getRuntime().addShutdownHook(shutdownThread); + shutdownAction = new SDKShutdownAction(); + Runtime.getRuntime().addShutdownHook(new Thread(shutdownAction, SDKShutdownActivity.class.getSimpleName())); } catch (Exception e) { InternalLogger.INSTANCE.error("Error while adding shutdown hook in getShutDownThread call"); InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e)); @@ -145,6 +176,6 @@ private SDKShutdownThread getShutdownThread() { } } - return shutdownThread; + return shutdownAction; } } diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/util/ThreadPoolUtils.java b/core/src/main/java/com/microsoft/applicationinsights/internal/util/ThreadPoolUtils.java index 2ad0da698c5..ef1c3be02e6 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/util/ThreadPoolUtils.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/util/ThreadPoolUtils.java @@ -22,9 +22,10 @@ package com.microsoft.applicationinsights.internal.util; import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; - +import java.util.concurrent.atomic.AtomicInteger; import com.microsoft.applicationinsights.internal.logger.InternalLogger; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -61,4 +62,30 @@ public static void stop(ThreadPoolExecutor threadPool, long timeout, TimeUnit ti Thread.currentThread().interrupt(); } } + + /** + * {@code poolName} will be appended with a hyphen and the threadId. + * @param clazz The class holding the thread pool + * @param instanceId The identifier of the instance of {@code clazz} + */ + public static ThreadFactory createDaemonThreadFactory(final Class clazz, final int instanceId) { + return createNamedDaemonThreadFactory(String.format("%s_%d", clazz.getSimpleName(), instanceId)); + } + + public static ThreadFactory createDaemonThreadFactory(final Class clazz) { + return createNamedDaemonThreadFactory(clazz.getSimpleName()); + } + + public static ThreadFactory createNamedDaemonThreadFactory(final String poolName) { + return new ThreadFactory(){ + private AtomicInteger threadId = new AtomicInteger(); + @Override + public Thread newThread(Runnable r) { + Thread thread = new Thread(r); + thread.setName(String.format("%s-%d", poolName, threadId.getAndIncrement())); + thread.setDaemon(true); + return thread; + } + }; + } } diff --git a/web/build.gradle b/web/build.gradle index 59869e84cc0..6da36825c87 100644 --- a/web/build.gradle +++ b/web/build.gradle @@ -36,7 +36,7 @@ dependencies { compile ([group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.1.3']) provided 'com.opensymphony:xwork:2.0.4' // Struts 2 provided 'org.springframework:spring-webmvc:3.1.0.RELEASE' - provided group: 'javax.servlet', name: 'servlet-api', version: '2.5' + provided group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1' provided group: 'javax.enterprise', name: 'cdi-api', version: '1.1' // Java EE testCompile group: 'junit', name: 'junit', version: '4.12' testCompile group: 'org.mockito', name: 'mockito-all', version: '1.8.0' diff --git a/web/src/main/java/com/microsoft/applicationinsights/web/internal/ApplicationInsightsServletContextListener.java b/web/src/main/java/com/microsoft/applicationinsights/web/internal/ApplicationInsightsServletContextListener.java new file mode 100644 index 00000000000..3f7ca7e43a0 --- /dev/null +++ b/web/src/main/java/com/microsoft/applicationinsights/web/internal/ApplicationInsightsServletContextListener.java @@ -0,0 +1,24 @@ +package com.microsoft.applicationinsights.web.internal; + +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; +import javax.servlet.annotation.WebListener; + +import com.microsoft.applicationinsights.internal.logger.InternalLogger; +import com.microsoft.applicationinsights.internal.shutdown.SDKShutdownActivity; +import com.microsoft.applicationinsights.internal.shutdown.Stoppable; + +@WebListener +public class ApplicationInsightsServletContextListener implements ServletContextListener { + + @Override + public void contextInitialized(ServletContextEvent sce) { + } + + @Override + public void contextDestroyed(ServletContextEvent sce) { + InternalLogger.INSTANCE.trace("Shutting down threads"); + SDKShutdownActivity.INSTANCE.stopAll(); + } + +} \ No newline at end of file diff --git a/web/src/main/java/com/microsoft/applicationinsights/web/internal/WebRequestTrackingFilter.java b/web/src/main/java/com/microsoft/applicationinsights/web/internal/WebRequestTrackingFilter.java index 6ff3e779ab2..09b2ce86d7c 100644 --- a/web/src/main/java/com/microsoft/applicationinsights/web/internal/WebRequestTrackingFilter.java +++ b/web/src/main/java/com/microsoft/applicationinsights/web/internal/WebRequestTrackingFilter.java @@ -36,6 +36,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.annotation.WebFilter; import com.microsoft.applicationinsights.common.CommonUtils; import com.microsoft.applicationinsights.TelemetryClient; @@ -48,6 +49,7 @@ /** * Created by yonisha on 2/2/2015. */ +@WebFilter public final class WebRequestTrackingFilter implements Filter { private final static String FILTER_NAME = "ApplicationInsightsWebFilter"; private final static String WEB_INF_FOLDER = "WEB-INF/"; diff --git a/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/AppProfileFetcher.java b/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/AppProfileFetcher.java index 80f294ff8c7..f80cc18d04c 100644 --- a/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/AppProfileFetcher.java +++ b/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/AppProfileFetcher.java @@ -21,14 +21,16 @@ package com.microsoft.applicationinsights.web.internal.correlation; +import java.io.Closeable; import java.io.IOException; import java.util.concurrent.ExecutionException; + import org.apache.http.ParseException; /** * Retrieves the application profile from storage */ - public interface AppProfileFetcher { + public interface AppProfileFetcher extends Closeable { /** * Fetches the application profile and returns the appId corresponding to the * instrumentation key provided. diff --git a/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/CdsProfileFetcher.java b/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/CdsProfileFetcher.java index d85daaeb274..9ba7de29ec1 100644 --- a/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/CdsProfileFetcher.java +++ b/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/CdsProfileFetcher.java @@ -28,6 +28,13 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import com.microsoft.applicationinsights.internal.logger.InternalLogger; +import com.microsoft.applicationinsights.internal.shutdown.SDKShutdownActivity; +import com.microsoft.applicationinsights.internal.shutdown.Stoppable; + +import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.http.HttpResponse; import org.apache.http.ParseException; import org.apache.http.client.config.RequestConfig; @@ -39,7 +46,7 @@ public class CdsProfileFetcher implements AppProfileFetcher { - private HttpAsyncClient httpClient; + private CloseableHttpAsyncClient httpClient; private String endpointAddress; private static final String ProfileQueryEndpointAppIdFormat = "%s/api/profiles/%s/appId"; private static final String DefaultProfileQueryEndpointAddress = "https://dc.services.visualstudio.com"; @@ -54,11 +61,11 @@ public CdsProfileFetcher() { .setConnectionRequestTimeout(5000) .build(); - this.httpClient = HttpAsyncClients.custom() + setHttpClient(HttpAsyncClients.custom() .setDefaultRequestConfig(requestConfig) - .build(); + .build()); - ((CloseableHttpAsyncClient)this.httpClient).start(); + this.httpClient.start(); this.tasks = new ConcurrentHashMap>(); this.endpointAddress = DefaultProfileQueryEndpointAddress; @@ -108,8 +115,9 @@ public ProfileFetcherResult fetchAppProfile(String instrumentationKey) throws In } } - public void setHttpClient(HttpAsyncClient client) { + public void setHttpClient(CloseableHttpAsyncClient client) { this.httpClient = client; + SDKShutdownActivity.INSTANCE.register(this.httpClient); } public void setEndpointAddress(String endpoint) throws MalformedURLException { @@ -124,4 +132,9 @@ private Future createFetchTask(String instrumentationKey) { HttpGet request = new HttpGet(String.format(ProfileQueryEndpointAppIdFormat, this.endpointAddress, instrumentationKey)); return this.httpClient.execute(request, null); } + + @Override + public void close() throws IOException { + this.httpClient.close(); + } } \ No newline at end of file diff --git a/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockHttpAsyncClientWrapper.java b/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockHttpAsyncClientWrapper.java index 1df461ed083..c54554fd477 100644 --- a/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockHttpAsyncClientWrapper.java +++ b/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockHttpAsyncClientWrapper.java @@ -21,9 +21,9 @@ package com.microsoft.applicationinsights.web.internal.correlation.mocks; -import org.apache.http.nio.client.HttpAsyncClient; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.concurrent.FutureCallback; +import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; import static org.mockito.Matchers.*; import static org.mockito.Mockito.mock; @@ -31,7 +31,7 @@ public class MockHttpAsyncClientWrapper { - private final HttpAsyncClient mockClient; + private final CloseableHttpAsyncClient mockClient; private final MockHttpEntity entity; private final MockHttpResponse response; private final MockHttpTask task; @@ -43,7 +43,7 @@ public MockHttpAsyncClientWrapper() { this.task = new MockHttpTask(this.response); - this.mockClient = mock(HttpAsyncClient.class); + this.mockClient = mock(CloseableHttpAsyncClient.class); when(mockClient.execute(any(HttpUriRequest.class), any(FutureCallback.class))).thenReturn(this.task); } @@ -68,7 +68,7 @@ public void setStatusCode(int code) { this.response.setStatusCode(code); } - public HttpAsyncClient getClient() { + public CloseableHttpAsyncClient getClient() { return this.mockClient; } } \ No newline at end of file diff --git a/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockProfileFetcher.java b/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockProfileFetcher.java index 5fc1af582c2..4517d08cba9 100644 --- a/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockProfileFetcher.java +++ b/web/src/test/java/com/microsoft/applicationinsights/web/internal/correlation/mocks/MockProfileFetcher.java @@ -21,6 +21,7 @@ package com.microsoft.applicationinsights.web.internal.correlation.mocks; +import java.io.IOException; import java.util.concurrent.ExecutionException; import com.microsoft.applicationinsights.web.internal.correlation.AppProfileFetcher; import com.microsoft.applicationinsights.web.internal.correlation.ProfileFetcherResultTaskStatus; @@ -59,4 +60,9 @@ public int callCount() { public void setResultStatus(ProfileFetcherResultTaskStatus status) { this.status = status; } + + @Override + public void close() throws IOException { + // nop + } } \ No newline at end of file