openkylin
/
platform_system_core
mirror of https://gitee.com/openkylin/platform_system_core.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

Prepare to fail in RefBase destructor if count is untouched 

Move towards crashing if a normally configured RefBase object is
destroyed without ever incrementing the reference count. We've been
threatening to do this for a long time. The previously last known
violation had been fixed.

This also fixes stack trace printing from RefBase, which had previously
been broken, and which we found necessary to track down further
violations of this rule.

Unfortunately, we found several more violations with the aid of
that fix. After existing CLs are submitted, there are
still some failures, but they are no longer numerous. Thus this CL
doesn't actually crash in the event of a violation, but does log a
verbose stack trace if it encounters one.

Bugs have been filed against the remaining known RefBase client offenders.
We plan to enable crashing on usage violations once those are fixed.

The fix for the stack trace printing breakage unfortunately requires
the use of weak symbols in order to avoid a circular build dependency.
We expect to eventually replace this with execinfo.h functionality.

Some random reformatting, driven by consistency with current formatting
requirements.

Add missing include to BacktraceMap.h.

Bug: 79112958
Bug: 30292291
Test: Boot AOSP, Master
Change-Id: I8151c54560c3b6f75ffc4c48229f0388a2066958
This commit is contained in:
Hans Boehm 2018-04-27 16:35:18 -07:00
parent f6b823141e
commit 9d3146af22
4 changed files with 121 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
libbacktrace/include/backtrace/BacktraceMap.h
View File

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

29
libutils/CallStack.cpp
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,26 @@ 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.
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;
}
}; // namespace android

57
libutils/RefBase.cpp
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;

64
libutils/include/utils/CallStack.h
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,11 @@
#include <stdint.h>
#include <sys/types.h>
#ifndef CALLSTACK_WEAK
#define CALLSTACK_WEAK __attribute__((weak))
#endif
#define ALWAYS_INLINE __attribute__((always_inline))
namespace android {
class Printer;
@ -36,7 +43,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 +51,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 +70,58 @@ 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 {
void operator()(CallStack* stack) { deleteStack(stack); }
};
typedef std::unique_ptr<CallStack, StackDeleter> CallStackUPtr;
// Return current call stack if possible, nullptr otherwise.
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);
}
}
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");
}
}
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>");
}
}
private:
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);
Vector<String8> mFrameLines;
};