Revert "Detect leaks of TextureBufferImpl objects."
This reverts commit 44bd29a3b068363e013cd425c68fd00dba21d633. Reason for revert: Going for an alternative implementation that makes this unnecessary https://webrtc-review.googlesource.com/c/src/+/150649 Original change's description: > Detect leaks of TextureBufferImpl objects. > > The performance cost is not trivial but according to my profiling, > it is acceptable. > > Bug: b/139745386 > Change-Id: I0e63221ccf22e9f6fb32c630ff63a279e765994a > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150539 > Reviewed-by: Kári Helgason <kthelgason@webrtc.org> > Commit-Queue: Sami Kalliomäki <sakal@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#28973} TBR=sakal@webrtc.org,kthelgason@webrtc.org Change-Id: Ic6266e5fd24389d41a6d5dbfe51de6505b861b12 Bug: b/139745386 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150650 Commit-Queue: Sami Kalliomäki <sakal@webrtc.org> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org> Cr-Commit-Position: refs/heads/master@{#28983}
This commit is contained in:

committed by
Commit Bot

parent
050e38f7c4
commit
fdd2340311
@ -19,8 +19,6 @@ import android.support.annotation.Nullable;
|
||||
* release callback. ToI420() is implemented by providing a Handler and a YuvConverter.
|
||||
*/
|
||||
public class TextureBufferImpl implements VideoFrame.TextureBuffer {
|
||||
private static final int RELEASE_TIMEOUT_MS = 10000;
|
||||
|
||||
// This is the full resolution the texture has in memory after applying the transformation matrix
|
||||
// that might include cropping. This resolution is useful to know when sampling the texture to
|
||||
// avoid downscaling artifacts.
|
||||
@ -62,7 +60,7 @@ public class TextureBufferImpl implements VideoFrame.TextureBuffer {
|
||||
this.transformMatrix = transformMatrix;
|
||||
this.toI420Handler = toI420Handler;
|
||||
this.yuvConverter = yuvConverter;
|
||||
this.refCountDelegate = new RefCountDelegate(releaseCallback, RELEASE_TIMEOUT_MS);
|
||||
this.refCountDelegate = new RefCountDelegate(releaseCallback);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -10,13 +10,7 @@
|
||||
|
||||
package org.webrtc;
|
||||
|
||||
import android.os.Handler;
|
||||
import android.os.Looper;
|
||||
import android.support.annotation.GuardedBy;
|
||||
import android.support.annotation.Nullable;
|
||||
import java.lang.ref.WeakReference;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
/**
|
||||
@ -25,44 +19,16 @@ import java.util.concurrent.atomic.AtomicInteger;
|
||||
class RefCountDelegate implements RefCounted {
|
||||
private final AtomicInteger refCount = new AtomicInteger(1);
|
||||
private final @Nullable Runnable releaseCallback;
|
||||
private final @Nullable RefCountMonitor refCountMonitor;
|
||||
|
||||
/**
|
||||
* Initializes a new ref count. The initial ref count will be 1.
|
||||
*
|
||||
* @param releaseCallback Callback that will be executed once the ref count reaches zero.
|
||||
*/
|
||||
public RefCountDelegate(@Nullable Runnable releaseCallback) {
|
||||
this(releaseCallback, /*releaseTimeoutMs=*/0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Initializes a new ref count with a release timeout. The initial ref count will be 1.
|
||||
*
|
||||
* @param releaseCallback Callback that will be executed once the ref count reaches zero.
|
||||
* @param releaseTimeoutMs If release timeout is not 0, release of this object will monitored.
|
||||
* When timeout is reached, stack traces for all threads that have called retain/release will
|
||||
* be printed.
|
||||
*/
|
||||
public RefCountDelegate(@Nullable Runnable releaseCallback, int releaseTimeoutMs) {
|
||||
if (releaseTimeoutMs < 0) {
|
||||
throw new IllegalArgumentException("Release timeout must be positive.");
|
||||
}
|
||||
|
||||
this.releaseCallback = releaseCallback;
|
||||
if (releaseTimeoutMs != 0) {
|
||||
refCountMonitor = new RefCountMonitor(this, releaseTimeoutMs);
|
||||
refCountMonitor.storeCurrentStackTrace();
|
||||
} else {
|
||||
refCountMonitor = null;
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void retain() {
|
||||
if (refCountMonitor != null) {
|
||||
refCountMonitor.storeCurrentStackTrace();
|
||||
}
|
||||
int updated_count = refCount.incrementAndGet();
|
||||
if (updated_count < 2) {
|
||||
throw new IllegalStateException("retain() called on an object with refcount < 1");
|
||||
@ -71,88 +37,12 @@ class RefCountDelegate implements RefCounted {
|
||||
|
||||
@Override
|
||||
public void release() {
|
||||
if (refCountMonitor != null) {
|
||||
refCountMonitor.storeCurrentStackTrace();
|
||||
}
|
||||
int updated_count = refCount.decrementAndGet();
|
||||
if (updated_count < 0) {
|
||||
throw new IllegalStateException("release() called on an object with refcount < 1");
|
||||
}
|
||||
if (updated_count == 0 && releaseCallback != null) {
|
||||
if (refCountMonitor != null) {
|
||||
refCountMonitor.cancel();
|
||||
}
|
||||
releaseCallback.run();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void finalize() {
|
||||
if (refCount.get() != 0) {
|
||||
Logging.e(toString(), "Leaked ref counted object with active references.");
|
||||
if (refCountMonitor != null) {
|
||||
refCountMonitor.printStackTraces(toString());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static final class StackTraceHolder {
|
||||
final String threadName;
|
||||
// A trick to store a stack trace (fast) is to construct a throwable.
|
||||
final Throwable throwable;
|
||||
|
||||
StackTraceHolder(String threadName, Throwable throwable) {
|
||||
this.threadName = threadName;
|
||||
this.throwable = throwable;
|
||||
}
|
||||
}
|
||||
|
||||
private static final class RefCountMonitor {
|
||||
@GuardedBy("stackTraces") private final List<StackTraceHolder> stackTraces = new ArrayList<>();
|
||||
|
||||
private final Runnable releaseTimeoutRunnable = this::onReleaseTimeout;
|
||||
private final WeakReference<RefCountDelegate> refCountDelegate;
|
||||
private final int releaseTimeoutMs;
|
||||
private final Handler releaseTimeoutHandler;
|
||||
|
||||
RefCountMonitor(RefCountDelegate refCountDelegate, int releaseTimeoutMs) {
|
||||
this.refCountDelegate = new WeakReference<>(refCountDelegate);
|
||||
this.releaseTimeoutMs = releaseTimeoutMs;
|
||||
this.releaseTimeoutHandler = new Handler(Looper.getMainLooper());
|
||||
|
||||
releaseTimeoutHandler.postDelayed(releaseTimeoutRunnable, releaseTimeoutMs);
|
||||
}
|
||||
|
||||
private void onReleaseTimeout() {
|
||||
final RefCountDelegate refCountDelegate = this.refCountDelegate.get();
|
||||
if (refCountDelegate == null) {
|
||||
return;
|
||||
}
|
||||
if (refCountDelegate.refCount.get() == 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
Logging.e(refCountDelegate.toString(), "Still unreleased ref counted object.");
|
||||
printStackTraces(refCountDelegate.toString());
|
||||
releaseTimeoutHandler.postDelayed(releaseTimeoutRunnable, releaseTimeoutMs);
|
||||
}
|
||||
|
||||
void printStackTraces(String tag) {
|
||||
synchronized (stackTraces) {
|
||||
for (StackTraceHolder stackTrace : stackTraces) {
|
||||
Logging.e(tag, "Stack trace for: " + stackTrace.threadName, stackTrace.throwable);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void cancel() {
|
||||
releaseTimeoutHandler.removeCallbacks(releaseTimeoutRunnable);
|
||||
}
|
||||
|
||||
void storeCurrentStackTrace() {
|
||||
synchronized (stackTraces) {
|
||||
stackTraces.add(new StackTraceHolder(Thread.currentThread().getName(), new Throwable()));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user