From 7a4c83922e551506fdc6c5fbca6d107ebdeef867 Mon Sep 17 00:00:00 2001 From: The Android Open Source Project Date: Thu, 5 Mar 2009 14:34:35 -0800 Subject: [PATCH] auto import from //depot/cupcake/@136594 --- include/utils/threads.h | 35 ------ libs/utils/CallStack.cpp | 25 +++- libs/utils/ResourceTypes.cpp | 14 ++- libs/utils/String16.cpp | 2 +- libs/utils/Threads.cpp | 226 ++++++++--------------------------- 5 files changed, 78 insertions(+), 224 deletions(-) diff --git a/include/utils/threads.h b/include/utils/threads.h index 7dca81004..8d8d46a05 100644 --- a/include/utils/threads.h +++ b/include/utils/threads.h @@ -248,41 +248,6 @@ private: }; -/* - * Read/write lock. The resource can have multiple readers or one writer, - * but can't be read and written at the same time. - * - * The same thread should not call a lock function while it already has - * a lock. (Should be okay for multiple readers.) - */ -class ReadWriteLock { -public: - ReadWriteLock() - : mNumReaders(0), mNumWriters(0) - {} - ~ReadWriteLock() {} - - void lockForRead(); - bool tryLockForRead(); - void unlockForRead(); - - void lockForWrite(); - bool tryLockForWrite(); - void unlockForWrite(); - -private: - int mNumReaders; - int mNumWriters; - - Mutex mLock; - Condition mReadWaiter; - Condition mWriteWaiter; -#if defined(PRINT_RENDER_TIMES) - DurationTimer mDebugTimer; -#endif -}; - - /* * This is our spiffy thread object! */ diff --git a/libs/utils/CallStack.cpp b/libs/utils/CallStack.cpp index 26fb22abc..2fdaa7118 100644 --- a/libs/utils/CallStack.cpp +++ b/libs/utils/CallStack.cpp @@ -120,13 +120,18 @@ class MapInfo { char name[]; }; - const char *map_to_name(uint64_t pc, const char* def) { + const char *map_to_name(uint64_t pc, const char* def, uint64_t* start) { mapinfo* mi = getMapInfoList(); while(mi) { - if ((pc >= mi->start) && (pc < mi->end)) + if ((pc >= mi->start) && (pc < mi->end)) { + if (start) + *start = mi->start; return mi->name; + } mi = mi->next; } + if (start) + *start = 0; return def; } @@ -183,8 +188,15 @@ public: } } - static const char *mapAddressToName(const void* pc, const char* def) { - return sMapInfo.map_to_name((uint64_t)pc, def); + static const char *mapAddressToName(const void* pc, const char* def, + void const** start) + { + uint64_t s; + char const* name = sMapInfo.map_to_name(uint64_t(uintptr_t(pc)), def, &s); + if (start) { + *start = (void*)s; + } + return name; } }; @@ -297,8 +309,9 @@ String8 CallStack::toStringSingleLevel(const char* prefix, int32_t level) const res.append(name); res.append(tmp2); } else { - name = MapInfo::mapAddressToName(ip, ""); - snprintf(tmp, 256, "pc %p %s", ip, name); + void const* start = 0; + name = MapInfo::mapAddressToName(ip, "", &start); + snprintf(tmp, 256, "pc %08lx %s", uintptr_t(ip)-uintptr_t(start), name); res.append(tmp); } res.append("\n"); diff --git a/libs/utils/ResourceTypes.cpp b/libs/utils/ResourceTypes.cpp index 71e7cd722..2ad3bfe8b 100644 --- a/libs/utils/ResourceTypes.cpp +++ b/libs/utils/ResourceTypes.cpp @@ -176,7 +176,9 @@ size_t Res_png_9patch::serializedSize() void* Res_png_9patch::serialize() { - void* newData = malloc(serializedSize()); + // Use calloc since we're going to leave a few holes in the data + // and want this to run cleanly under valgrind + void* newData = calloc(1, serializedSize()); serialize(newData); return newData; } @@ -3150,13 +3152,13 @@ bool ResTable::stringToValue(Res_value* outValue, String16* outString, const char16_t* pos = s; while (pos < end && !failed) { const char16_t* start = pos; - end++; + pos++; while (pos < end && *pos != '|') { pos++; } - //printf("Looking for: %s\n", String8(start, pos-start).string()); + //printf("Looking for: %s\n", String8(start, pos-start).string()); const bag_entry* bagi = bag; - ssize_t i; + ssize_t i; for (i=0; imap.name.ident)) { //printf("Trying attr #%08x\n", bagi->map.name.ident); @@ -3184,7 +3186,7 @@ bool ResTable::stringToValue(Res_value* outValue, String16* outString, } unlockBag(bag); if (!failed) { - //printf("Final flag value: 0x%lx\n", outValue->data); + //printf("Final flag value: 0x%lx\n", outValue->data); return true; } } @@ -3192,7 +3194,7 @@ bool ResTable::stringToValue(Res_value* outValue, String16* outString, if (fromAccessor) { if (accessor->getAttributeFlags(attrID, s, len, outValue)) { - //printf("Final flag value: 0x%lx\n", outValue->data); + //printf("Final flag value: 0x%lx\n", outValue->data); return true; } } diff --git a/libs/utils/String16.cpp b/libs/utils/String16.cpp index 1f81cadb7..ae0ae1e09 100644 --- a/libs/utils/String16.cpp +++ b/libs/utils/String16.cpp @@ -388,7 +388,7 @@ status_t String16::setTo(const char16_t* other, size_t len) ->editResize((len+1)*sizeof(char16_t)); if (buf) { char16_t* str = (char16_t*)buf->data(); - memcpy(str, other, len*sizeof(char16_t)); + memmove(str, other, len*sizeof(char16_t)); str[len] = 0; mString = str; return NO_ERROR; diff --git a/libs/utils/Threads.cpp b/libs/utils/Threads.cpp index 5f407a990..9287c0bba 100644 --- a/libs/utils/Threads.cpp +++ b/libs/utils/Threads.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +// #define LOG_NDEBUG 0 #define LOG_TAG "libutils.threads" #include @@ -838,148 +839,6 @@ void Condition::broadcast() #error "condition variables not supported on this platform" #endif - -/* - * =========================================================================== - * ReadWriteLock class - * =========================================================================== - */ - -#if 0 -#pragma mark - -#pragma mark ReadWriteLock -#endif - -/* - * Add a reader. Readers are nice. They share. - */ -void ReadWriteLock::lockForRead() -{ - mLock.lock(); - while (mNumWriters > 0) { - LOG(LOG_DEBUG, "thread", "+++ lockForRead: waiting\n"); - mReadWaiter.wait(mLock); - } - assert(mNumWriters == 0); - mNumReaders++; -#if defined(PRINT_RENDER_TIMES) - if (mNumReaders == 1) - mDebugTimer.start(); -#endif - mLock.unlock(); -} - -/* - * Try to add a reader. If it doesn't work right away, return "false". - */ -bool ReadWriteLock::tryLockForRead() -{ - mLock.lock(); - if (mNumWriters > 0) { - mLock.unlock(); - return false; - } - assert(mNumWriters == 0); - mNumReaders++; -#if defined(PRINT_RENDER_TIMES) - if (mNumReaders == 1) - mDebugTimer.start(); -#endif - mLock.unlock(); - return true; -} - -/* - * Remove a reader. - */ -void ReadWriteLock::unlockForRead() -{ - mLock.lock(); - if (mNumReaders == 0) { - mLock.unlock(); - LOG(LOG_WARN, "thread", - "WARNING: unlockForRead requested, but not locked\n"); - return; - } - assert(mNumReaders > 0); - assert(mNumWriters == 0); - mNumReaders--; - if (mNumReaders == 0) { // last reader? -#if defined(PRINT_RENDER_TIMES) - mDebugTimer.stop(); - printf(" rdlk held %.3f msec\n", - (double) mDebugTimer.durationUsecs() / 1000.0); -#endif - //printf("+++ signaling writers (if any)\n"); - mWriteWaiter.signal(); // wake one writer (if any) - } - mLock.unlock(); -} - -/* - * Add a writer. This requires exclusive access to the object. - */ -void ReadWriteLock::lockForWrite() -{ - mLock.lock(); - while (mNumReaders > 0 || mNumWriters > 0) { - LOG(LOG_DEBUG, "thread", "+++ lockForWrite: waiting\n"); - mWriteWaiter.wait(mLock); - } - assert(mNumReaders == 0); - assert(mNumWriters == 0); - mNumWriters++; -#if defined(PRINT_RENDER_TIMES) - mDebugTimer.start(); -#endif - mLock.unlock(); -} - -/* - * Try to add a writer. If it doesn't work right away, return "false". - */ -bool ReadWriteLock::tryLockForWrite() -{ - mLock.lock(); - if (mNumReaders > 0 || mNumWriters > 0) { - mLock.unlock(); - return false; - } - assert(mNumReaders == 0); - assert(mNumWriters == 0); - mNumWriters++; -#if defined(PRINT_RENDER_TIMES) - mDebugTimer.start(); -#endif - mLock.unlock(); - return true; -} - -/* - * Remove a writer. - */ -void ReadWriteLock::unlockForWrite() -{ - mLock.lock(); - if (mNumWriters == 0) { - mLock.unlock(); - LOG(LOG_WARN, "thread", - "WARNING: unlockForWrite requested, but not locked\n"); - return; - } - assert(mNumWriters == 1); - mNumWriters--; -#if defined(PRINT_RENDER_TIMES) - mDebugTimer.stop(); - //printf(" wrlk held %.3f msec\n", - // (double) mDebugTimer.durationUsecs() / 1000.0); -#endif - mWriteWaiter.signal(); // should other writers get first dibs? - //printf("+++ signaling readers (if any)\n"); - mReadWaiter.broadcast(); // wake all readers (if any) - mLock.unlock(); -} - // ---------------------------------------------------------------------------- #if 0 @@ -1027,6 +886,8 @@ status_t Thread::run(const char* name, int32_t priority, size_t stack) // hold a strong reference on ourself mHoldSelf = this; + mRunning = true; + bool res; if (mCanCallJava) { res = createThreadEtc(_threadLoop, @@ -1040,14 +901,16 @@ status_t Thread::run(const char* name, int32_t priority, size_t stack) mStatus = UNKNOWN_ERROR; // something happened! mRunning = false; mThread = thread_id_t(-1); + mHoldSelf.clear(); // "this" may have gone away after this. + + return UNKNOWN_ERROR; } - if (mStatus < 0) { - // something happened, don't leak - mHoldSelf.clear(); - } - - return mStatus; + // Do not refer to mStatus here: The thread is already running (may, in fact + // already have exited with a valid mStatus result). The NO_ERROR indication + // here merely indicates successfully starting the thread and does not + // imply successful termination/execution. + return NO_ERROR; } int Thread::_threadLoop(void* user) @@ -1057,20 +920,32 @@ int Thread::_threadLoop(void* user) wp weak(strong); self->mHoldSelf.clear(); - // we're about to run... - self->mStatus = self->readyToRun(); - if (self->mStatus!=NO_ERROR || self->mExitPending) { - // pretend the thread never started... - self->mExitPending = false; - self->mRunning = false; - return 0; - } - - // thread is running now - self->mRunning = true; + bool first = true; do { - bool result = self->threadLoop(); + bool result; + if (first) { + first = false; + self->mStatus = self->readyToRun(); + result = (self->mStatus == NO_ERROR); + + if (result && !self->mExitPending) { + // Binder threads (and maybe others) rely on threadLoop + // running at least once after a successful ::readyToRun() + // (unless, of course, the thread has already been asked to exit + // at that point). + // This is because threads are essentially used like this: + // (new ThreadSubclass())->run(); + // The caller therefore does not retain a strong reference to + // the thread and the thread would simply disappear after the + // successful ::readyToRun() call instead of entering the + // threadLoop at least once. + result = self->threadLoop(); + } + } else { + result = self->threadLoop(); + } + if (result == false || self->mExitPending) { self->mExitPending = true; self->mLock.lock(); @@ -1097,24 +972,23 @@ void Thread::requestExit() status_t Thread::requestExitAndWait() { - if (mStatus == OK) { + if (mThread == getThreadId()) { + LOGW( + "Thread (this=%p): don't call waitForExit() from this " + "Thread object's thread. It's a guaranteed deadlock!", + this); - if (mThread == getThreadId()) { - LOGW( - "Thread (this=%p): don't call waitForExit() from this " - "Thread object's thread. It's a guaranteed deadlock!", - this); - return WOULD_BLOCK; - } - - requestExit(); - - Mutex::Autolock _l(mLock); - while (mRunning == true) { - mThreadExitedCondition.wait(mLock); - } - mExitPending = false; + return WOULD_BLOCK; } + + requestExit(); + + Mutex::Autolock _l(mLock); + while (mRunning == true) { + mThreadExitedCondition.wait(mLock); + } + mExitPending = false; + return mStatus; }