Revert^2 "Prepare to fail in RefBase destructor if count is untouched"
This reverts commit b9d0753d2ba88cc60947823e68bb3bed60268361.
Reason for revert: Re-land with MacOS workaround.
Test: Build (on Linux) and boot AOSP, with weak symbols enabled and disabled.
Change-Id: I5150cd90367178f3b039761dca3bccc9c2987df1
diff --git a/libutils/RefBase.cpp b/libutils/RefBase.cpp
index 9074850..3f1e79a 100644
--- a/libutils/RefBase.cpp
+++ b/libutils/RefBase.cpp
@@ -17,30 +17,41 @@
#define LOG_TAG "RefBase"
// #define LOG_NDEBUG 0
+#include <memory>
+
#include <utils/RefBase.h>
#include <utils/CallStack.h>
+#include <utils/Mutex.h>
+
#ifndef __unused
#define __unused __attribute__((__unused__))
#endif
-// compile with refcounting debugging enabled
-#define DEBUG_REFS 0
+// Compile with refcounting debugging enabled.
+#define DEBUG_REFS 0
+
+// The following three are ignored unless DEBUG_REFS is set.
// whether ref-tracking is enabled by default, if not, trackMe(true, false)
// needs to be called explicitly
-#define DEBUG_REFS_ENABLED_BY_DEFAULT 0
+#define DEBUG_REFS_ENABLED_BY_DEFAULT 0
// whether callstack are collected (significantly slows things down)
-#define DEBUG_REFS_CALLSTACK_ENABLED 1
+#define DEBUG_REFS_CALLSTACK_ENABLED 1
// folder where stack traces are saved when DEBUG_REFS is enabled
// this folder needs to exist and be writable
-#define DEBUG_REFS_CALLSTACK_PATH "/data/debug"
+#define DEBUG_REFS_CALLSTACK_PATH "/data/debug"
// log all reference counting operations
-#define PRINT_REFS 0
+#define PRINT_REFS 0
+
+// Continue after logging a stack trace if ~RefBase discovers that reference
+// count has never been incremented. Normally we conspicuously crash in that
+// case.
+#define DEBUG_REFBASE_DESTRUCTION 1
// ---------------------------------------------------------------------------
@@ -184,7 +195,7 @@
char inc = refs->ref >= 0 ? '+' : '-';
ALOGD("\t%c ID %p (ref %d):", inc, refs->id, refs->ref);
#if DEBUG_REFS_CALLSTACK_ENABLED
- refs->stack.log(LOG_TAG);
+ CallStack::logStack(LOG_TAG, refs->stack.get());
#endif
refs = refs->next;
}
@@ -198,14 +209,14 @@
char inc = refs->ref >= 0 ? '+' : '-';
ALOGD("\t%c ID %p (ref %d):", inc, refs->id, refs->ref);
#if DEBUG_REFS_CALLSTACK_ENABLED
- refs->stack.log(LOG_TAG);
+ CallStack::logStack(LOG_TAG, refs->stack.get());
#endif
refs = refs->next;
}
}
if (dumpStack) {
ALOGE("above errors at:");
- CallStack stack(LOG_TAG);
+ CallStack::logStack(LOG_TAG);
}
}
@@ -279,7 +290,7 @@
this);
int rc = open(name, O_RDWR | O_CREAT | O_APPEND, 644);
if (rc >= 0) {
- write(rc, text.string(), text.length());
+ (void)write(rc, text.string(), text.length());
close(rc);
ALOGD("STACK TRACE for %p saved in %s", this, name);
}
@@ -294,7 +305,7 @@
ref_entry* next;
const void* id;
#if DEBUG_REFS_CALLSTACK_ENABLED
- CallStack stack;
+ CallStack::CallStackUPtr stack;
#endif
int32_t ref;
};
@@ -311,7 +322,7 @@
ref->ref = mRef;
ref->id = id;
#if DEBUG_REFS_CALLSTACK_ENABLED
- ref->stack.update(2);
+ ref->stack = CallStack::getCurrent(2);
#endif
ref->next = *refs;
*refs = ref;
@@ -346,7 +357,7 @@
ref = ref->next;
}
- CallStack stack(LOG_TAG);
+ CallStack::logStack(LOG_TAG);
}
}
@@ -373,7 +384,7 @@
inc, refs->id, refs->ref);
out->append(buf);
#if DEBUG_REFS_CALLSTACK_ENABLED
- out->append(refs->stack.toString("\t\t"));
+ out->append(CallStack::stackToString("\t\t", refs->stack.get()));
#else
out->append("\t\t(call stacks disabled)");
#endif
@@ -700,16 +711,16 @@
if (mRefs->mWeak.load(std::memory_order_relaxed) == 0) {
delete mRefs;
}
- } else if (mRefs->mStrong.load(std::memory_order_relaxed)
- == INITIAL_STRONG_VALUE) {
+ } else if (mRefs->mStrong.load(std::memory_order_relaxed) == INITIAL_STRONG_VALUE) {
// We never acquired a strong reference on this object.
- LOG_ALWAYS_FATAL_IF(mRefs->mWeak.load() != 0,
- "RefBase: Explicit destruction with non-zero weak "
- "reference count");
- // TODO: Always report if we get here. Currently MediaMetadataRetriever
- // C++ objects are inconsistently managed and sometimes get here.
- // There may be other cases, but we believe they should all be fixed.
- delete mRefs;
+#if DEBUG_REFBASE_DESTRUCTION
+ // Treating this as fatal is prone to causing boot loops. For debugging, it's
+ // better to treat as non-fatal.
+ ALOGD("RefBase: Explicit destruction, weak count = %d (in %p)", mRefs->mWeak.load(), this);
+ CallStack::logStack(LOG_TAG);
+#else
+ LOG_ALWAYS_FATAL("RefBase: Explicit destruction, weak count = %d", mRefs->mWeak.load());
+#endif
}
// For debugging purposes, clear mRefs. Ineffective against outstanding wp's.
const_cast<weakref_impl*&>(mRefs) = nullptr;