andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Deprecate and ignore ResourceLeakDetector's maxActive parameter

Browse Source 
Motivation:
ResourceLeakDetector supports a parameter called maxActive. This parameter is used in attempt to limit the amount of objects which are being tracked for leaks at any given time, and generates an error log message if this limit is exceeded. This assumes that there is a relationship between leak sample rate and object lifetime for objects which are already being tracked. This relationship may appear to work in cases were there are a single leak record per object and those leak records live for the lifetime of the application but in general this relationship doesn't exist. The original motivation was to provide a limit for cases such as HashedWheelTimer to limit the number of instances which exist at any given time. This limit is not enforced in all circumstances in HashedWheelTimer (e.g. if the thread is a daemon) and can be implemented outside ResourceLeakDetector.

Modifications:
- Deprecate all methods which interact with maxActive in ResourceLeakDetectorFactory and ResourceLeakDetector
- Remove all logic related to maxActive in ResourceLeakDetector
- HashedWheelTimer implements its own logic to impose a limit and warn users if too many instances exists at any given time.

Result:
Fixes https://github.com/netty/netty/issues/6225.
This commit is contained in:
Scott Mitchell 2017-01-18 10:56:25 -08:00
parent ccd1d77714
commit 14b902fced
3 changed files with 161 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
common/src/main/java/io/netty/util/HashedWheelTimer.java
View File

@ -29,9 +29,13 @@ import java.util.concurrent.Executors;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
import java.util.concurrent.atomic.AtomicLong;
import static io.netty.util.internal.StringUtil.simpleClassName;
/**
* A {@link Timer} optimized for approximated I/O timeout scheduling.
*
@ -78,8 +82,11 @@ public class HashedWheelTimer implements Timer {
static final InternalLogger logger =
InternalLoggerFactory.getInstance(HashedWheelTimer.class);
private static final AtomicInteger INSTANCE_COUNTER = new AtomicInteger();
private static final AtomicBoolean WARNED_TOO_MANY_INSTANCES = new AtomicBoolean();
private static final int INSTANCE_COUNT_LIMIT = 64;
private static final ResourceLeakDetector<HashedWheelTimer> leakDetector = ResourceLeakDetectorFactory.instance()
.newResourceLeakDetector(HashedWheelTimer.class, 1, Runtime.getRuntime().availableProcessors() * 4L);
.newResourceLeakDetector(HashedWheelTimer.class, 1);
private static final AtomicIntegerFieldUpdater<HashedWheelTimer> WORKER_STATE_UPDATER =
AtomicIntegerFieldUpdater.newUpdater(HashedWheelTimer.class, "workerState");
@ -266,6 +273,24 @@ public class HashedWheelTimer implements Timer {
leak = leakDetection || !workerThread.isDaemon() ? leakDetector.track(this) : null;
this.maxPendingTimeouts = maxPendingTimeouts;
if (INSTANCE_COUNTER.incrementAndGet() > INSTANCE_COUNT_LIMIT &&
WARNED_TOO_MANY_INSTANCES.compareAndSet(false, true)) {
reportTooManyInstances();
}
}
@Override
protected void finalize() throws Throwable {
try {
super.finalize();
} finally {
// This object is going to be GCed and it is assumed the ship has sailed to do a proper shutdown. If
// we have not yet shutdown then we want to make sure we decrement the active instance count.
if (WORKER_STATE_UPDATER.getAndSet(this, WORKER_STATE_SHUTDOWN) != WORKER_STATE_SHUTDOWN) {
INSTANCE_COUNTER.decrementAndGet();
}
}
}
private static HashedWheelBucket[] createWheel(int ticksPerWheel) {
@ -337,33 +362,37 @@ public class HashedWheelTimer implements Timer {
if (!WORKER_STATE_UPDATER.compareAndSet(this, WORKER_STATE_STARTED, WORKER_STATE_SHUTDOWN)) {
// workerState can be 0 or 2 at this moment - let it always be 2.
WORKER_STATE_UPDATER.set(this, WORKER_STATE_SHUTDOWN);
if (leak != null) {
boolean closed = leak.close(this);
assert closed;
if (WORKER_STATE_UPDATER.getAndSet(this, WORKER_STATE_SHUTDOWN) != WORKER_STATE_SHUTDOWN) {
INSTANCE_COUNTER.decrementAndGet();
if (leak != null) {
boolean closed = leak.close(this);
assert closed;
}
}
return Collections.emptySet();
}
boolean interrupted = false;
while (workerThread.isAlive()) {
workerThread.interrupt();
try {
workerThread.join(100);
} catch (InterruptedException ignored) {
interrupted = true;
try {
boolean interrupted = false;
while (workerThread.isAlive()) {
workerThread.interrupt();
try {
workerThread.join(100);
} catch (InterruptedException ignored) {
interrupted = true;
}
}
}
if (interrupted) {
Thread.currentThread().interrupt();
}
if (leak != null) {
boolean closed = leak.close(this);
assert closed;
if (interrupted) {
Thread.currentThread().interrupt();
}
} finally {
INSTANCE_COUNTER.decrementAndGet();
if (leak != null) {
boolean closed = leak.close(this);
assert closed;
}
}
return worker.unprocessedTimeouts();
}
@ -400,6 +429,13 @@ public class HashedWheelTimer implements Timer {
return maxPendingTimeouts > 0;
}
private static void reportTooManyInstances() {
String resourceType = simpleClassName(HashedWheelTimer.class);
logger.error("You are creating too many " + resourceType + " instances. " +
resourceType + " is a shared resource that must be reused across the JVM," +
"so that only a few instances are created.");
}
private final class Worker implements Runnable {
private final Set<Timeout> unprocessedTimeouts = new HashSet<Timeout>();
@ -636,7 +672,7 @@ public class HashedWheelTimer implements Timer {
long remaining = deadline - currentTime + timer.startTime;
StringBuilder buf = new StringBuilder(192)
.append(StringUtil.simpleClassName(this))
.append(simpleClassName(this))
.append('(')
.append("deadline: ");
if (remaining > 0) {

39
common/src/main/java/io/netty/util/ResourceLeakDetector.java
View File

@ -161,8 +161,6 @@ public class ResourceLeakDetector<T> {
private final String resourceType;
private final int samplingInterval;
private final long maxActive;
private final AtomicBoolean loggedTooManyActive = new AtomicBoolean();
/**
* @deprecated use {@link ResourceLeakDetectorFactory#newResourceLeakDetector(Class, int, long)}.
@ -180,31 +178,43 @@ public class ResourceLeakDetector<T> {
this(resourceType, DEFAULT_SAMPLING_INTERVAL, Long.MAX_VALUE);
}
/**
* @deprecated Use {@link ResourceLeakDetector#ResourceLeakDetector(Class, int)}.
* <p>
* This should not be used directly by users of {@link ResourceLeakDetector}.
* Please use {@link ResourceLeakDetectorFactory#newResourceLeakDetector(Class)}
* or {@link ResourceLeakDetectorFactory#newResourceLeakDetector(Class, int, long)}
*
* @param maxActive This is deprecated and will be ignored.
*/
@Deprecated
public ResourceLeakDetector(Class<?> resourceType, int samplingInterval, long maxActive) {
this(resourceType, samplingInterval);
}
/**
* This should not be used directly by users of {@link ResourceLeakDetector}.
* Please use {@link ResourceLeakDetectorFactory#newResourceLeakDetector(Class)}
* or {@link ResourceLeakDetectorFactory#newResourceLeakDetector(Class, int, long)}
*/
@SuppressWarnings("deprecation")
public ResourceLeakDetector(Class<?> resourceType, int samplingInterval, long maxActive) {
this(simpleClassName(resourceType), samplingInterval, maxActive);
public ResourceLeakDetector(Class<?> resourceType, int samplingInterval) {
this(simpleClassName(resourceType), samplingInterval, Long.MAX_VALUE);
}
/**
* @deprecated use {@link ResourceLeakDetectorFactory#newResourceLeakDetector(Class, int, long)}.
* <p>
* @param maxActive This is deprecated and will be ignored.
*/
@Deprecated
public ResourceLeakDetector(String resourceType, int samplingInterval, long maxActive) {
if (resourceType == null) {
throw new NullPointerException("resourceType");
}
if (maxActive <= 0) {
throw new IllegalArgumentException("maxActive: " + maxActive + " (expected: 1+)");
}
this.resourceType = resourceType;
this.samplingInterval = samplingInterval;
this.maxActive = maxActive;
}
/**
@ -261,12 +271,6 @@ public class ResourceLeakDetector<T> {
return;
}
// Report too many instances.
int samplingInterval = level == Level.PARANOID? 1 : this.samplingInterval;
if (allLeaks.size() * samplingInterval > maxActive && loggedTooManyActive.compareAndSet(false, true)) {
reportInstancesLeak(resourceType);
}
// Detect and report previous leaks.
for (;;) {
@SuppressWarnings("unchecked")
@ -317,13 +321,10 @@ public class ResourceLeakDetector<T> {
}
/**
* This method is called when instance leaks are detected. It can be overridden for tracking how many times leaks
* have been detected.
* @deprecated This method will no longer be invoked by {@link ResourceLeakDetector}.
*/
@Deprecated
protected void reportInstancesLeak(String resourceType) {
logger.error("LEAK: You are creating too many " + resourceType + " instances. " +
resourceType + " is a shared resource that must be reused across the JVM," +
"so that only a few instances are created.");
}
@SuppressWarnings("deprecation")

104
common/src/main/java/io/netty/util/ResourceLeakDetectorFactory.java
View File

@ -37,7 +37,7 @@ public abstract class ResourceLeakDetectorFactory {
/**
* Get the singleton instance of this factory class.
*
* @return - the current {@link ResourceLeakDetectorFactory}
* @return the current {@link ResourceLeakDetectorFactory}
*/
public static ResourceLeakDetectorFactory instance() {
return factoryInstance;
@ -48,7 +48,7 @@ public abstract class ResourceLeakDetectorFactory {
* {@link ResourceLeakDetector} is called by all the callers of this factory. That is, before initializing a
* Netty Bootstrap.
*
* @param factory - the instance that will become the current {@link ResourceLeakDetectorFactory}'s singleton
* @param factory the instance that will become the current {@link ResourceLeakDetectorFactory}'s singleton
*/
public static void setResourceLeakDetectorFactory(ResourceLeakDetectorFactory factory) {
factoryInstance = ObjectUtil.checkNotNull(factory, "factory");
@ -57,30 +57,47 @@ public abstract class ResourceLeakDetectorFactory {
/**
* Returns a new instance of a {@link ResourceLeakDetector} with the given resource class.
*
* @param resource - the resource class used to initialize the {@link ResourceLeakDetector}
* @param <T> - the type of the resource class
* @return - a new instance of {@link ResourceLeakDetector}
* @param resource the resource class used to initialize the {@link ResourceLeakDetector}
* @param <T> the type of the resource class
* @return a new instance of {@link ResourceLeakDetector}
*/
public final <T> ResourceLeakDetector<T> newResourceLeakDetector(Class<T> resource) {
return newResourceLeakDetector(resource, ResourceLeakDetector.DEFAULT_SAMPLING_INTERVAL, Long.MAX_VALUE);
return newResourceLeakDetector(resource, ResourceLeakDetector.DEFAULT_SAMPLING_INTERVAL);
}
/**
* @deprecated Use {@link #newResourceLeakDetector(Class, int)} instead.
* <p>
* Returns a new instance of a {@link ResourceLeakDetector} with the given resource class.
*
* @param resource the resource class used to initialize the {@link ResourceLeakDetector}
* @param samplingInterval the interval on which sampling takes place
* @param maxActive This is deprecated and will be ignored.
* @param <T> the type of the resource class
* @return a new instance of {@link ResourceLeakDetector}
*/
@Deprecated
public abstract <T> ResourceLeakDetector<T> newResourceLeakDetector(
Class<T> resource, int samplingInterval, long maxActive);
/**
* Returns a new instance of a {@link ResourceLeakDetector} with the given resource class.
*
* @param resource - the resource class used to initialize the {@link ResourceLeakDetector}
* @param samplingInterval - the interval on which sampling takes place
* @param maxActive - the maximum active instances
* @param <T> - the type of the resource class
* @return - a new instance of {@link ResourceLeakDetector}
* @param resource the resource class used to initialize the {@link ResourceLeakDetector}
* @param samplingInterval the interval on which sampling takes place
* @param <T> the type of the resource class
* @return a new instance of {@link ResourceLeakDetector}
*/
public abstract <T> ResourceLeakDetector<T> newResourceLeakDetector(
Class<T> resource, int samplingInterval, long maxActive);
@SuppressWarnings("deprecation")
public <T> ResourceLeakDetector<T> newResourceLeakDetector(Class<T> resource, int samplingInterval) {
return newResourceLeakDetector(resource, ResourceLeakDetector.DEFAULT_SAMPLING_INTERVAL, Long.MAX_VALUE);
}
/**
* Default implementation that loads custom leak detector via system property
*/
private static final class DefaultResourceLeakDetectorFactory extends ResourceLeakDetectorFactory {
private final Constructor<?> obsoleteCustomClassConstructor;
private final Constructor<?> customClassConstructor;
DefaultResourceLeakDetectorFactory() {
@ -96,10 +113,15 @@ public abstract class ResourceLeakDetectorFactory {
logger.error("Could not access System property: io.netty.customResourceLeakDetector", cause);
customLeakDetector = null;
}
customClassConstructor = customLeakDetector == null ? null : customClassConstructor(customLeakDetector);
if (customLeakDetector == null) {
obsoleteCustomClassConstructor = customClassConstructor = null;
} else {
obsoleteCustomClassConstructor = obsoleteCustomClassConstructor(customLeakDetector);
customClassConstructor = customClassConstructor(customLeakDetector);
}
}
private static Constructor<?> customClassConstructor(String customLeakDetector) {
private static Constructor<?> obsoleteCustomClassConstructor(String customLeakDetector) {
try {
final Class<?> detectorClass = Class.forName(customLeakDetector, true,
PlatformDependent.getSystemClassLoader());
@ -116,15 +138,56 @@ public abstract class ResourceLeakDetectorFactory {
return null;
}
private static Constructor<?> customClassConstructor(String customLeakDetector) {
try {
final Class<?> detectorClass = Class.forName(customLeakDetector, true,
PlatformDependent.getSystemClassLoader());
if (ResourceLeakDetector.class.isAssignableFrom(detectorClass)) {
return detectorClass.getConstructor(Class.class, int.class);
} else {
logger.error("Class {} does not inherit from ResourceLeakDetector.", customLeakDetector);
}
} catch (Throwable t) {
logger.error("Could not load custom resource leak detector class provided: {}",
customLeakDetector, t);
}
return null;
}
@SuppressWarnings("deprecation")
@Override
public <T> ResourceLeakDetector<T> newResourceLeakDetector(
Class<T> resource, int samplingInterval, long maxActive) {
public <T> ResourceLeakDetector<T> newResourceLeakDetector(Class<T> resource, int samplingInterval,
long maxActive) {
if (obsoleteCustomClassConstructor != null) {
try {
@SuppressWarnings("unchecked")
ResourceLeakDetector<T> leakDetector =
(ResourceLeakDetector<T>) obsoleteCustomClassConstructor.newInstance(
resource, samplingInterval, maxActive);
logger.debug("Loaded custom ResourceLeakDetector: {}",
obsoleteCustomClassConstructor.getDeclaringClass().getName());
return leakDetector;
} catch (Throwable t) {
logger.error(
"Could not load custom resource leak detector provided: {} with the given resource: {}",
obsoleteCustomClassConstructor.getDeclaringClass().getName(), resource, t);
}
}
ResourceLeakDetector<T> resourceLeakDetector = new ResourceLeakDetector<T>(resource, samplingInterval,
maxActive);
logger.debug("Loaded default ResourceLeakDetector: {}", resourceLeakDetector);
return resourceLeakDetector;
}
@Override
public <T> ResourceLeakDetector<T> newResourceLeakDetector(Class<T> resource, int samplingInterval) {
if (customClassConstructor != null) {
try {
@SuppressWarnings("unchecked")
ResourceLeakDetector<T> leakDetector =
(ResourceLeakDetector<T>) customClassConstructor.newInstance(
resource, samplingInterval, maxActive);
(ResourceLeakDetector<T>) customClassConstructor.newInstance(resource, samplingInterval);
logger.debug("Loaded custom ResourceLeakDetector: {}",
customClassConstructor.getDeclaringClass().getName());
return leakDetector;
@ -135,8 +198,7 @@ public abstract class ResourceLeakDetectorFactory {
}
}
ResourceLeakDetector<T> resourceLeakDetector = new ResourceLeakDetector<T>(
resource, samplingInterval, maxActive);
ResourceLeakDetector<T> resourceLeakDetector = new ResourceLeakDetector<T>(resource, samplingInterval);
logger.debug("Loaded default ResourceLeakDetector: {}", resourceLeakDetector);
return resourceLeakDetector;
}