Skip to content

Commit

Permalink
Always enable leak tracking for derived buffers if parent is tracked (#…
Browse files Browse the repository at this point in the history
…13436)

Motivation:

Currently, when leak tracking is set to SIMPLE or ADVANCED, and a
derived buffer is created, the buffer is selected for leak tracking if
the parent buffer is tracked and the derived buffer is also selected by
an independent roll of the dice. As a result, deeply derived buffers are
exponentially less likely to be selected for leak tracking. This makes
it difficult to find and diagnose leaks related to such buffers.

Modifications:

When a derived buffer is created, and the parent buffer has leak
tracking, the derived buffer now always has leak tracking applied.

Result:

Leaks of derived buffers are easier to find. Fixes #13414.
  • Loading branch information
rdicroce committed Jun 13, 2023
1 parent bdb7873 commit 7d5fcf7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,8 @@ private ByteBuf unwrappedDerived(ByteBuf derived) {
// Update the parent to point to this buffer so we correctly close the ResourceLeakTracker.
((AbstractPooledDerivedByteBuf) unwrappedDerived).parent(this);

ResourceLeakTracker<ByteBuf> newLeak = AbstractByteBuf.leakDetector.track(derived);
if (newLeak == null) {
// No leak detection, just return the derived buffer.
return derived;
}
return newLeakAwareByteBuf(derived, newLeak);
// force tracking of derived buffers (see issue #13414)
return newLeakAwareByteBuf(derived, AbstractByteBuf.leakDetector.trackForcibly(derived));
}
return newSharedLeakAwareByteBuf(derived);
}
Expand Down
38 changes: 23 additions & 15 deletions common/src/main/java/io/netty/util/ResourceLeakDetector.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public ResourceLeakDetector(String resourceType, int samplingInterval, long maxA
*/
@Deprecated
public final ResourceLeak open(T obj) {
return track0(obj);
return track0(obj, false);
}

/**
Expand All @@ -242,25 +242,33 @@ public final ResourceLeak open(T obj) {
*/
@SuppressWarnings("unchecked")
public final ResourceLeakTracker<T> track(T obj) {
return track0(obj);
return track0(obj, false);
}

/**
* Creates a new {@link ResourceLeakTracker} which is expected to be closed via
* {@link ResourceLeakTracker#close(Object)} when the related resource is deallocated.
*
* Unlike {@link #track(Object)}, this method always returns a tracker, regardless
* of the detection settings.
*
* @return the {@link ResourceLeakTracker}
*/
@SuppressWarnings("unchecked")
private DefaultResourceLeak track0(T obj) {
Level level = ResourceLeakDetector.level;
if (level == Level.DISABLED) {
return null;
}
public ResourceLeakTracker<T> trackForcibly(T obj) {
return track0(obj, true);
}

if (level.ordinal() < Level.PARANOID.ordinal()) {
if ((PlatformDependent.threadLocalRandom().nextInt(samplingInterval)) == 0) {
reportLeak();
return new DefaultResourceLeak(obj, refQueue, allLeaks, getInitialHint(resourceType));
}
return null;
@SuppressWarnings("unchecked")
private DefaultResourceLeak track0(T obj, boolean force) {
Level level = ResourceLeakDetector.level;
if (force ||
level == Level.PARANOID ||
(level != Level.DISABLED && PlatformDependent.threadLocalRandom().nextInt(samplingInterval) == 0)) {
reportLeak();
return new DefaultResourceLeak(obj, refQueue, allLeaks, getInitialHint(resourceType));
}
reportLeak();
return new DefaultResourceLeak(obj, refQueue, allLeaks, getInitialHint(resourceType));
return null;
}

private void clearRefQueue() {
Expand Down

0 comments on commit 7d5fcf7

Please sign in to comment.