From 6d9089f9788e9dd3e975dee2bd02580f7ed4c88e Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 12 Jun 2017 11:36:18 +0200 Subject: Remove finalizer in ActiveContainer No instances of the log message has been detected the last month, which indicates that the finalizer is not necessary. --- .../java/com/yahoo/jdisc/core/ActiveContainer.java | 19 ------ .../core/ActiveContainerDeactivationWatchdog.java | 9 +-- .../jdisc/core/ActiveContainerFinalizerTest.java | 75 ---------------------- 3 files changed, 1 insertion(+), 102 deletions(-) delete mode 100644 jdisc_core/src/test/java/com/yahoo/jdisc/core/ActiveContainerFinalizerTest.java (limited to 'jdisc_core/src') 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 51a84a4a6c6..fbc45f000d2 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 @@ -17,15 +17,12 @@ import com.yahoo.jdisc.service.ServerProvider; import java.net.URI; import java.util.Map; -import java.util.logging.Logger; /** * @author Simon Thoresen */ 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 serverProviders; @@ -67,22 +64,6 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine termination.run(); } - // TODO Get rid of finalizer and use PhantomReference or Java 9 Cleaner instead - @Override - protected void finalize() throws Throwable { - try { - int retainCount = retainCount(); - if (retainCount > 0) { - log.severe("Destructing " + this + " through finalizer since reference count never reached zero. " + - "This is an indication of either a resource leak or invalid use of reference counting. " + - "Retained references as this moment: " + retainCount); - destroy(); - } - } finally { - super.finalize(); - } - } - /** * Make this instance retain a reference to the resource until it is destroyed. */ diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainerDeactivationWatchdog.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainerDeactivationWatchdog.java index a6b4ef03c61..0004d1d818b 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainerDeactivationWatchdog.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainerDeactivationWatchdog.java @@ -60,7 +60,7 @@ class ActiveContainerDeactivationWatchdog implements ActiveContainerMetrics, Aut WATCHDOG_FREQUENCY.getSeconds(), TimeUnit.SECONDS); this.scheduler.scheduleAtFixedRate( - ActiveContainerDeactivationWatchdog::triggerGc, + System::gc, GC_TRIGGER_FREQUENCY.getSeconds(), GC_TRIGGER_FREQUENCY.getSeconds(), TimeUnit.SECONDS); @@ -107,13 +107,6 @@ class ActiveContainerDeactivationWatchdog implements ActiveContainerMetrics, Aut } } - private static void triggerGc() { - // ActiveContainer has a finalizer, so gc -> finalizer -> gc is required. - System.gc(); - System.runFinalization(); - System.gc(); - } - private List getDeactivatedContainersSnapshot() { Instant now = clock.instant(); synchronized (monitor) { diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ActiveContainerFinalizerTest.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ActiveContainerFinalizerTest.java deleted file mode 100644 index b2fd357b30c..00000000000 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ActiveContainerFinalizerTest.java +++ /dev/null @@ -1,75 +0,0 @@ -// 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.jdisc.Container; -import com.yahoo.jdisc.Request; -import com.yahoo.jdisc.test.TestDriver; - -import java.net.URI; -import java.util.concurrent.TimeUnit; - -import org.junit.Test; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; - -/** - * @author Simon Thoresen Hult - */ -@SuppressWarnings("UnusedAssignment") -public class ActiveContainerFinalizerTest { - - @Test - public void requireThatMissingContainerReleaseDoesNotPreventShutdown() throws InterruptedException { - final TestDriver driver = TestDriver.newSimpleApplicationInstanceWithoutOsgi(); - driver.activateContainer(driver.newContainerBuilder()); - Container container = driver.newReference(URI.create("scheme://host")); - assertNotNull(container); - - final Termination termination = new Termination(); - driver.activateContainer(null).notifyTermination(termination); - assertFalse(termination.await(100, TimeUnit.MILLISECONDS)); - - container = null; // intentionally doing this instead of container.release() - assertTrue(termination.await(600, TimeUnit.SECONDS)); - assertTrue(driver.close()); - } - - @Test - public void requireThatMissingRequestReleaseDoesNotPreventShutdown() throws InterruptedException { - final TestDriver driver = TestDriver.newSimpleApplicationInstanceWithoutOsgi(); - driver.activateContainer(driver.newContainerBuilder()); - Request request = new Request(driver, URI.create("scheme://host")); - assertNotNull(request); - - final Termination termination = new Termination(); - driver.activateContainer(null).notifyTermination(termination); - assertFalse(termination.await(100, TimeUnit.MILLISECONDS)); - - request = null; // intentionally doing this instead of request.release() - assertTrue(termination.await(600, TimeUnit.SECONDS)); - assertTrue(driver.close()); - } - - private static class Termination implements Runnable { - - volatile boolean done; - - @Override - public void run() { - done = true; - } - - boolean await(final int timeout, final TimeUnit unit) throws InterruptedException { - final long timeoutAt = System.currentTimeMillis() + unit.toMillis(timeout); - while (!done) { - if (System.currentTimeMillis() > timeoutAt) { - return false; - } - Thread.sleep(10); - System.gc(); - } - return true; - } - } -} -- cgit v1.2.3