diff options
Diffstat (limited to 'jdisc_core')
10 files changed, 321 insertions, 26 deletions
diff --git a/jdisc_core/pom.xml b/jdisc_core/pom.xml index 6227600f24c..bc6240919e9 100644 --- a/jdisc_core/pom.xml +++ b/jdisc_core/pom.xml @@ -120,6 +120,12 @@ <scope>compile</scope> </dependency> <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>vespajlib</artifactId> + <version>${project.version}</version> + <scope>compile</scope> + </dependency> + <dependency> <!-- This seems odd. Used for export-package parsing. Lazy stuff. Should be separated out. --> <groupId>com.yahoo.vespa</groupId> <artifactId>bundle-plugin</artifactId> diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java b/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java index 3ed10170420..82029e85cd6 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java @@ -69,6 +69,7 @@ public abstract class AbstractResource implements SharedResource { return new DebugResourceReference(this, referenceStack); } + @Override public void release() { initialCreationReference.close(); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java index a296bd1e327..692c669780f 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java @@ -9,7 +9,6 @@ import com.yahoo.jdisc.application.BindingSet; import com.yahoo.jdisc.application.BindingSetSelector; import com.yahoo.jdisc.application.ContainerBuilder; import com.yahoo.jdisc.application.ResourcePool; -import com.yahoo.jdisc.application.UriPattern; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.service.BindingSetNotFoundException; import com.yahoo.jdisc.service.CurrentContainer; @@ -18,12 +17,15 @@ import com.yahoo.jdisc.service.ServerProvider; import java.net.URI; import java.util.Map; +import java.util.logging.Logger; /** * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> */ public class ActiveContainer extends AbstractResource implements CurrentContainer { + private static final Logger log = Logger.getLogger(ActiveContainer.class.getName()); + private final ContainerTermination termination; private final Injector guiceInjector; private final Iterable<ServerProvider> serverProviders; @@ -35,21 +37,15 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine public ActiveContainer(ContainerBuilder builder) { serverProviders = builder.serverProviders().activate(); - for (SharedResource resource : serverProviders) { - resourceReferences.retain(resource); - } + serverProviders.forEach(resourceReferences::retain); serverBindings = builder.activateServerBindings(); - for (BindingSet<RequestHandler> set : serverBindings.values()) { - for (Map.Entry<UriPattern, RequestHandler> entry : set) { - resourceReferences.retain(entry.getValue()); - } - } + serverBindings.forEach( + (ignoredName, bindingSet) -> bindingSet.forEach( + binding -> resourceReferences.retain(binding.getValue()))); clientBindings = builder.activateClientBindings(); - for (BindingSet<RequestHandler> set : clientBindings.values()) { - for (Map.Entry<UriPattern, RequestHandler> entry : set) { - resourceReferences.retain(entry.getValue()); - } - } + clientBindings.forEach( + (ignoredName, bindingSet) -> bindingSet.forEach( + binding -> resourceReferences.retain(binding.getValue()))); bindingSetSelector = builder.getInstance(BindingSetSelector.class); timeoutMgr = builder.getInstance(TimeoutManagerImpl.class); timeoutMgr.start(); @@ -74,7 +70,11 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine @Override protected void finalize() throws Throwable { try { - if (retainCount() > 0) { + int retainCount = retainCount(); + if (retainCount > 0) { + log.warning(this + ".destroy() invoked from finalize() not through ApplicationLoader. " + + "This is an indication of either a resource leak or invalid use of reference counting. " + + "Retained references as this moment: " + retainCount); destroy(); } } finally { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java index c6d6efd0ee9..c6acde814eb 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java @@ -8,6 +8,7 @@ import com.yahoo.jdisc.application.ContainerBuilder; import com.yahoo.jdisc.application.ContainerThread; import com.yahoo.jdisc.application.OsgiFramework; import com.yahoo.jdisc.service.CurrentContainer; +import com.yahoo.jdisc.statistics.ActiveContainerStatistics; import java.util.concurrent.ThreadFactory; @@ -28,6 +29,7 @@ class ApplicationEnvironmentModule extends AbstractModule { bind(CurrentContainer.class).toInstance(loader); bind(OsgiFramework.class).toInstance(loader.osgiFramework()); bind(ThreadFactory.class).to(ContainerThread.Factory.class); + bind(ActiveContainerStatistics.class).toInstance(loader.getActiveContainerStatistics()); } @Provides diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java index 2dd7f7eb879..4e63bc77c9a 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java @@ -5,9 +5,17 @@ import com.google.inject.AbstractModule; import com.google.inject.Injector; import com.google.inject.Module; import com.yahoo.jdisc.AbstractResource; -import com.yahoo.jdisc.application.*; +import com.yahoo.jdisc.application.Application; +import com.yahoo.jdisc.application.ApplicationNotReadyException; +import com.yahoo.jdisc.application.ContainerActivator; +import com.yahoo.jdisc.application.ContainerBuilder; +import com.yahoo.jdisc.application.DeactivatedContainer; +import com.yahoo.jdisc.application.GuiceRepository; +import com.yahoo.jdisc.application.OsgiFramework; +import com.yahoo.jdisc.application.OsgiHeader; import com.yahoo.jdisc.service.ContainerNotReadyException; import com.yahoo.jdisc.service.CurrentContainer; +import com.yahoo.jdisc.statistics.ActiveContainerStatistics; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; @@ -28,11 +36,13 @@ import java.util.logging.Logger; public class ApplicationLoader implements BootstrapLoader, ContainerActivator, CurrentContainer { private static final Logger log = Logger.getLogger(ApplicationLoader.class.getName()); + private final OsgiFramework osgiFramework; private final GuiceRepository guiceModules = new GuiceRepository(); private final AtomicReference<ActiveContainer> containerRef = new AtomicReference<>(); private final Object appLock = new Object(); private final List<Bundle> appBundles = new ArrayList<>(); + private final ActiveContainerStatistics statistics = new ActiveContainerStatistics(); private Application application; private ApplicationInUseTracker applicationInUseTracker; @@ -62,9 +72,11 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C } prev = containerRef.getAndSet(next); + statistics.onActivated(next); if (prev == null) { return null; } + statistics.onDeactivated(prev); } prev.release(); DeactivatedContainer deactivatedContainer = prev.shutdown(); @@ -82,11 +94,9 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C Thread.sleep(TimeUnit.MILLISECONDS.convert(currentWaitTimeSeconds, TimeUnit.SECONDS)) ); + statistics.printSummaryToLog(); final ActiveContainer prevContainer = prevContainerReference.get(); - if (prevContainer == null) { - return; - } - if (prevContainer.retainCount() == 0) { + if (prevContainer == null || prevContainer.retainCount() == 0) { return; } log.warning("Previous container not terminated in the last " + totalTimeWaited + " seconds." @@ -231,6 +241,10 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C } } + public ActiveContainerStatistics getActiveContainerStatistics() { + return statistics; + } + public OsgiFramework osgiFramework() { return osgiFramework; } @@ -258,4 +272,5 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C } } } + } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/BootstrapDaemon.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/BootstrapDaemon.java index 03110782d23..33d03d29f7d 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/BootstrapDaemon.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/BootstrapDaemon.java @@ -1,10 +1,13 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.core; +import com.yahoo.protect.Process; import org.apache.commons.daemon.Daemon; import org.apache.commons.daemon.DaemonContext; import java.util.Arrays; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,6 +40,56 @@ public class BootstrapDaemon implements Daemon { return loader; } + private static class WatchDog implements Runnable { + final String name; + final CountDownLatch complete; + final long timeout; + final TimeUnit timeUnit; + WatchDog(String name, CountDownLatch complete, long timeout, TimeUnit timeUnit) { + this.name = name; + this.complete = complete; + this.timeout = timeout; + this.timeUnit = timeUnit; + } + @Override + public void run() { + boolean dumpStack; + try { + dumpStack = !complete.await(timeout, timeUnit); + } catch (InterruptedException e) { + return; + } + if (dumpStack) { + log.warning("The watchdog for BootstrapDaemon." + name + " detected that it had not completed in " + + timeUnit.toMillis(timeout) + "ms. Dumping stack."); + Process.dumpThreads(); + } + } + } + private interface MyRunnable { + void run() throws Exception; + } + private void startWithWatchDog(String name, long timeout, TimeUnit timeUnit, MyRunnable task) throws Exception { + CountDownLatch complete = new CountDownLatch(1); + Thread thread = new Thread(new WatchDog(name, complete, timeout, timeUnit), name); + thread.setDaemon(true); + thread.start(); + try { + task.run(); + } catch (Exception e) { + log.log(Level.WARNING, "Exception caught during BootstrapDaemon." + name, e); + throw e; + } catch (Error e) { + log.log(Level.WARNING, "Error caught during BootstrapDaemon." + name, e); + throw e; + } catch (Throwable thrown) { + log.log(Level.WARNING, "Throwable caught during BootstrapDaemon." + name, thrown); + } finally { + complete.countDown(); + thread.join(); + } + } + @Override public void init(DaemonContext context) throws Exception { String[] args = context.getArguments(); @@ -46,7 +99,7 @@ public class BootstrapDaemon implements Daemon { bundleLocation = args[0]; if (privileged) { log.finer("Initializing application with privileges."); - loader.init(bundleLocation, true); + startWithWatchDog("init", 60, TimeUnit.SECONDS, () -> loader.init(bundleLocation, true)); } } @@ -55,9 +108,9 @@ public class BootstrapDaemon implements Daemon { try { if (!privileged) { log.finer("Initializing application without privileges."); - loader.init(bundleLocation, false); + startWithWatchDog("init", 60, TimeUnit.SECONDS, () -> loader.init(bundleLocation, false)); } - loader.start(); + startWithWatchDog("start", 60, TimeUnit.SECONDS, () -> loader.start()); } catch (Exception e) { try { log.log(Level.SEVERE, "Failed starting container", e); @@ -70,12 +123,15 @@ public class BootstrapDaemon implements Daemon { @Override public void stop() throws Exception { - loader.stop(); + startWithWatchDog("stop", 60, TimeUnit.SECONDS, () -> loader.stop()); } @Override public void destroy() { - loader.destroy(); + try { + startWithWatchDog("destroy", 60, TimeUnit.SECONDS, () -> loader.destroy()); + } catch (Exception e) { + } } } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ExportPackages.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ExportPackages.java index afe43718bc5..3dbc4af5422 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ExportPackages.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ExportPackages.java @@ -1,7 +1,6 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.core; -import com.yahoo.container.plugin.bundle.AnalyzeBundle; import com.yahoo.container.plugin.bundle.TransformExportPackages; import com.yahoo.container.plugin.osgi.ExportPackages.Export; import org.apache.felix.framework.util.Util; @@ -38,6 +37,7 @@ public class ExportPackages { .append("com.yahoo.jdisc.application,") .append("com.yahoo.jdisc.handler,") .append("com.yahoo.jdisc.service,") + .append("com.yahoo.jdisc.statistics,") .append("javax.inject;version=1.0.0,") // Included in guice, but not exported. Needed by container-jersey. .append("org.aopalliance.intercept,") .append("org.aopalliance.aop,") diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ActiveContainerStatistics.java b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ActiveContainerStatistics.java new file mode 100644 index 00000000000..2fac3b4024b --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ActiveContainerStatistics.java @@ -0,0 +1,132 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.statistics; + +import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.core.ActiveContainer; + +import java.time.Instant; +import java.util.List; +import java.util.WeakHashMap; +import java.util.logging.Logger; +import java.util.stream.Stream; + +import static java.util.stream.Collectors.toList; + +/** + * Tracks statistics on stale {@link ActiveContainer} instances. + * + * @author bjorncs + */ +public class ActiveContainerStatistics { + public interface Metrics { + String TOTAL_DEACTIVATED_CONTAINERS = "jdisc.deactivated_containers.total"; + String DEACTIVATED_CONTAINERS_WITH_RETAINED_REFERENCES = "jdisc.deactivated_containers.with_retained_refs"; + } + + private static final Logger log = Logger.getLogger(ActiveContainerStatistics.class.getName()); + + private final WeakHashMap<ActiveContainer, ActiveContainerStats> activeContainers = new WeakHashMap<>(); + private final Object monitor = new Object(); + + public void emitMetrics(Metric metric) { + synchronized (monitor) { + DeactivatedContainerMetrics metrics = deactivatedContainerStream() + .collect( + DeactivatedContainerMetrics::new, + DeactivatedContainerMetrics::aggregate, + DeactivatedContainerMetrics::merge); + + metric.set(Metrics.TOTAL_DEACTIVATED_CONTAINERS, metrics.deactivatedContainerCount, null); + metric.set(Metrics.DEACTIVATED_CONTAINERS_WITH_RETAINED_REFERENCES, metrics.deactivatedContainersWithRetainedRefsCount, null); + } + } + + public void onActivated(ActiveContainer activeContainer) { + synchronized (monitor) { + activeContainers.put(activeContainer, new ActiveContainerStats(Instant.now())); + } + } + + public void onDeactivated(ActiveContainer activeContainer) { + synchronized (monitor) { + ActiveContainerStats containerStats = activeContainers.get(activeContainer); + if (containerStats == null) { + throw new IllegalStateException("onActivated() has not been called for container: " + activeContainer); + } + containerStats.timeDeactivated = Instant.now(); + } + } + + public void printSummaryToLog() { + synchronized (monitor) { + List<DeactivatedContainer> deactivatedContainers = deactivatedContainerStream().collect(toList()); + if (deactivatedContainers.isEmpty()) return; + + log.warning( + "Multiple instances of ActiveContainer leaked! " + deactivatedContainers.size() + + " instances are still present."); + deactivatedContainers.stream() + .map(c -> " - " + c.toSummaryString()) + .forEach(log::warning); + } + } + + private Stream<DeactivatedContainer> deactivatedContainerStream() { + synchronized (monitor) { + return activeContainers.entrySet().stream() + .filter(e -> e.getValue().isDeactivated()) + .map(e -> new DeactivatedContainer(e.getKey(), e.getValue().timeActivated, e.getValue().timeDeactivated)); + } + } + + private static class ActiveContainerStats { + public final Instant timeActivated; + public Instant timeDeactivated; + + public ActiveContainerStats(Instant timeActivated) { + this.timeActivated = timeActivated; + } + + public boolean isDeactivated() { + return timeDeactivated != null; + } + } + + private static class DeactivatedContainer { + public final ActiveContainer activeContainer; + public final Instant timeActivated; + public final Instant timeDeactivated; + + public DeactivatedContainer(ActiveContainer activeContainer, Instant timeActivated, Instant timeDeactivated) { + this.activeContainer = activeContainer; + this.timeActivated = timeActivated; + this.timeDeactivated = timeDeactivated; + } + + public String toSummaryString() { + return String.format("%s: timeActivated=%s, timeDeactivated=%s, retainCount=%d", + activeContainer.toString(), + timeActivated.toString(), + timeDeactivated.toString(), + activeContainer.retainCount()); + } + } + + private static class DeactivatedContainerMetrics { + public int deactivatedContainerCount = 0; + public int deactivatedContainersWithRetainedRefsCount = 0; + + public void aggregate(DeactivatedContainer deactivatedContainer) { + ++deactivatedContainerCount; + if (deactivatedContainer.activeContainer.retainCount() > 0) { + ++deactivatedContainersWithRetainedRefsCount; + } + } + + public DeactivatedContainerMetrics merge(DeactivatedContainerMetrics other) { + deactivatedContainerCount += other.deactivatedContainerCount; + deactivatedContainersWithRetainedRefsCount += other.deactivatedContainersWithRetainedRefsCount; + return this; + } + } +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java new file mode 100644 index 00000000000..4289bcbaa9d --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java @@ -0,0 +1,4 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +@com.yahoo.osgi.annotation.ExportPackage +package com.yahoo.jdisc.statistics; diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/statistics/ActiveContainerStatisticsTest.java b/jdisc_core/src/test/java/com/yahoo/jdisc/statistics/ActiveContainerStatisticsTest.java new file mode 100644 index 00000000000..0a3482b705a --- /dev/null +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/statistics/ActiveContainerStatisticsTest.java @@ -0,0 +1,79 @@ +package com.yahoo.jdisc.statistics; + +import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.ResourceReference; +import com.yahoo.jdisc.core.ActiveContainer; +import com.yahoo.jdisc.test.TestDriver; +import org.junit.Test; + +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +/** + * @author bjorncs + */ +public class ActiveContainerStatisticsTest { + + @Test + public void counts_deactivated_activecontainers() { + TestDriver driver = TestDriver.newSimpleApplicationInstanceWithoutOsgi(); + ActiveContainerStatistics stats = new ActiveContainerStatistics(); + MockMetric metric = new MockMetric(); + + ActiveContainer containerWithoutRetainedResources = new ActiveContainer(driver.newContainerBuilder()); + + stats.onActivated(containerWithoutRetainedResources); + stats.emitMetrics(metric); + assertEquals(0, metric.totalCount); + assertEquals(0, metric.withRetainedReferencesCount); + + stats.onDeactivated(containerWithoutRetainedResources); + containerWithoutRetainedResources.release(); + stats.emitMetrics(metric); + assertEquals(1, metric.totalCount); + assertEquals(0, metric.withRetainedReferencesCount); + + ActiveContainer containerWithRetainedResources = new ActiveContainer(driver.newContainerBuilder()); + + try (ResourceReference ignoredRef = containerWithRetainedResources.refer()) { + stats.onActivated(containerWithRetainedResources); + stats.onDeactivated(containerWithRetainedResources); + containerWithRetainedResources.release(); + stats.emitMetrics(metric); + assertEquals(2, metric.totalCount); + assertEquals(1, metric.withRetainedReferencesCount); + } + + } + + private static class MockMetric implements Metric { + public int totalCount; + public int withRetainedReferencesCount; + + @Override + public void set(String key, Number val, Context ctx) { + switch (key) { + case ActiveContainerStatistics.Metrics.TOTAL_DEACTIVATED_CONTAINERS: + totalCount = val.intValue(); + break; + case ActiveContainerStatistics.Metrics.DEACTIVATED_CONTAINERS_WITH_RETAINED_REFERENCES: + withRetainedReferencesCount = val.intValue(); + break; + default: + throw new UnsupportedOperationException(); + } + } + + @Override + public void add(String key, Number val, Context ctx) { + throw new UnsupportedOperationException(); + } + + @Override + public Context createContext(Map<String, ?> properties) { + throw new UnsupportedOperationException(); + } + } + +} |