Revert^2 "Prepare to fail in RefBase destructor if count is untouched"

This reverts commit b9d0753d2b.

Reason for revert: Re-land with MacOS workaround.

Test: Build (on Linux) and boot AOSP, with weak symbols enabled and disabled.

Change-Id: I5150cd90367178f3b039761dca3bccc9c2987df1
This commit is contained in:
Hans Boehm 2018-08-07 23:45:25 +00:00
parent 7d4e7d3156
commit 2a019ecf4f
4 changed files with 163 additions and 30 deletions

View file

@ -31,6 +31,7 @@
#include <deque>
#include <iterator>
#include <memory>
#include <string>
#include <vector>

View file

@ -16,16 +16,15 @@
#define LOG_TAG "CallStack"
#include <utils/CallStack.h>
#include <memory>
#include <utils/Printer.h>
#include <utils/Errors.h>
#include <utils/Log.h>
#include <backtrace/Backtrace.h>
#define CALLSTACK_WEAK // Don't generate weak definitions.
#include <utils/CallStack.h>
namespace android {
CallStack::CallStack() {
@ -76,4 +75,30 @@ void CallStack::print(Printer& printer) const {
}
}
// The following four functions may be used via weak symbol references from libutils.
// Clients assume that if any of these symbols are available, then deleteStack() is.
#ifdef WEAKS_AVAILABLE
CallStack::CallStackUPtr CallStack::getCurrentInternal(int ignoreDepth) {
CallStack::CallStackUPtr stack(new CallStack());
stack->update(ignoreDepth + 1);
return stack;
}
void CallStack::logStackInternal(const char* logtag, const CallStack* stack,
android_LogPriority priority) {
stack->log(logtag, priority);
}
String8 CallStack::stackToStringInternal(const char* prefix, const CallStack* stack) {
return stack->toString(prefix);
}
void CallStack::deleteStack(CallStack* stack) {
delete stack;
}
#endif // WEAKS_AVAILABLE
}; // namespace android

View file

@ -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 @@ public:
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 @@ public:
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 @@ public:
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 @@ private:
ref_entry* next;
const void* id;
#if DEBUG_REFS_CALLSTACK_ENABLED
CallStack stack;
CallStack::CallStackUPtr stack;
#endif
int32_t ref;
};
@ -311,7 +322,7 @@ private:
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 @@ private:
ref = ref->next;
}
CallStack stack(LOG_TAG);
CallStack::logStack(LOG_TAG);
}
}
@ -373,7 +384,7 @@ private:
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 @@ RefBase::~RefBase()
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;

View file

@ -17,6 +17,8 @@
#ifndef ANDROID_CALLSTACK_H
#define ANDROID_CALLSTACK_H
#include <memory>
#include <android/log.h>
#include <backtrace/backtrace_constants.h>
#include <utils/String8.h>
@ -25,6 +27,19 @@
#include <stdint.h>
#include <sys/types.h>
#if !defined(__APPLE__) && !defined(_WIN32)
# define WEAKS_AVAILABLE 1
#endif
#ifndef CALLSTACK_WEAK
# ifdef WEAKS_AVAILABLE
# define CALLSTACK_WEAK __attribute__((weak))
# else // !WEAKS_AVAILABLE
# define CALLSTACK_WEAK
# endif // !WEAKS_AVAILABLE
#endif // CALLSTACK_WEAK predefined
#define ALWAYS_INLINE __attribute__((always_inline))
namespace android {
class Printer;
@ -36,7 +51,7 @@ public:
CallStack();
// Create a callstack with the current thread's stack trace.
// Immediately dump it to logcat using the given logtag.
CallStack(const char* logtag, int32_t ignoreDepth=1);
CallStack(const char* logtag, int32_t ignoreDepth = 1);
~CallStack();
// Reset the stack frames (same as creating an empty call stack).
@ -44,7 +59,7 @@ public:
// Immediately collect the stack traces for the specified thread.
// The default is to dump the stack of the current call.
void update(int32_t ignoreDepth=1, pid_t tid=BACKTRACE_CURRENT_THREAD);
void update(int32_t ignoreDepth = 1, pid_t tid = BACKTRACE_CURRENT_THREAD);
// Dump a stack trace to the log using the supplied logtag.
void log(const char* logtag,
@ -63,7 +78,88 @@ public:
// Get the count of stack frames that are in this call stack.
size_t size() const { return mFrameLines.size(); }
private:
// DO NOT USE ANYTHING BELOW HERE. The following public members are expected
// to disappear again shortly, once a better replacement facility exists.
// The replacement facility will be incompatible!
// Debugging accesses to some basic functionality. These use weak symbols to
// avoid introducing a dependency on libutilscallstack. Such a dependency from
// libutils results in a cyclic build dependency. These routines can be called
// from within libutils. But if the actual library is unavailable, they do
// nothing.
//
// DO NOT USE THESE. They will disappear.
struct StackDeleter {
#ifdef WEAKS_AVAILABLE
void operator()(CallStack* stack) {
deleteStack(stack);
}
#else
void operator()(CallStack*) {}
#endif
};
typedef std::unique_ptr<CallStack, StackDeleter> CallStackUPtr;
// Return current call stack if possible, nullptr otherwise.
#ifdef WEAKS_AVAILABLE
static CallStackUPtr ALWAYS_INLINE getCurrent(int32_t ignoreDepth = 1) {
if (reinterpret_cast<uintptr_t>(getCurrentInternal) == 0) {
ALOGW("CallStack::getCurrentInternal not linked, returning null");
return CallStackUPtr(nullptr);
} else {
return getCurrentInternal(ignoreDepth);
}
}
#else // !WEAKS_AVAILABLE
static CallStackUPtr ALWAYS_INLINE getCurrent(int32_t = 1) {
return CallStackUPtr(nullptr);
}
#endif // !WEAKS_AVAILABLE
#ifdef WEAKS_AVAILABLE
static void ALWAYS_INLINE logStack(const char* logtag, CallStack* stack = getCurrent().get(),
android_LogPriority priority = ANDROID_LOG_DEBUG) {
if (reinterpret_cast<uintptr_t>(logStackInternal) != 0 && stack != nullptr) {
logStackInternal(logtag, stack, priority);
} else {
ALOGW("CallStack::logStackInternal not linked");
}
}
#else
static void ALWAYS_INLINE logStack(const char*, CallStack* = getCurrent().get(),
android_LogPriority = ANDROID_LOG_DEBUG) {
}
#endif // !WEAKS_AVAILABLE
#ifdef WEAKS_AVAILABLE
static String8 ALWAYS_INLINE stackToString(const char* prefix = nullptr,
const CallStack* stack = getCurrent().get()) {
if (reinterpret_cast<uintptr_t>(stackToStringInternal) != 0 && stack != nullptr) {
return stackToStringInternal(prefix, stack);
} else {
return String8("<CallStack package not linked>");
}
}
#else // !WEAKS_AVAILABLE
static String8 ALWAYS_INLINE stackToString(const char* = nullptr,
const CallStack* = getCurrent().get()) {
return String8("<CallStack package not linked>");
}
#endif // !WEAKS_AVAILABLE
private:
#ifdef WEAKS_AVAILABLE
static CallStackUPtr CALLSTACK_WEAK getCurrentInternal(int32_t ignoreDepth);
static void CALLSTACK_WEAK logStackInternal(const char* logtag, const CallStack* stack,
android_LogPriority priority);
static String8 CALLSTACK_WEAK stackToStringInternal(const char* prefix, const CallStack* stack);
// The deleter is only invoked on non-null pointers. Hence it will never be
// invoked if CallStack is not linked.
static void CALLSTACK_WEAK deleteStack(CallStack* stack);
#endif // WEAKS_AVAILABLE
Vector<String8> mFrameLines;
};