Merge changes I6555c135,If5723ff4

* changes:
  logcat: test: also check "failed to clear" in blocking_clear
  logd: reader/writer element locks

logcat/tests/logcat_test.cpp

@@ -1216,7 +1216,12 @@ TEST(logcat, blocking_clear) {
     signal(SIGALRM, caught_blocking_clear);
     alarm(2);
     while (fgets(buffer, sizeof(buffer), fp)) {
-        if (!strncmp(buffer, "clearLog: ", 10)) {
+        if (!strncmp(buffer, "clearLog: ", strlen("clearLog: "))) {
+            fprintf(stderr, "WARNING: Test lacks permission to run :-(\n");
+            count = signals = 1;
+            break;
+        }
+        if (!strncmp(buffer, "failed to clear", strlen("failed to clear"))) {
             fprintf(stderr, "WARNING: Test lacks permission to run :-(\n");
             count = signals = 1;
             break;

logd/FlushCommand.cpp

@@ -44,14 +44,14 @@ FlushCommand::FlushCommand(LogReader& reader, bool nonBlock, unsigned long tail,
 // LogTimeEntrys, and spawn a transitory per-client thread to
 // work at filing data to the  socket.
 //
-// global LogTimeEntry::lock() is used to protect access,
+// global LogTimeEntry::wrlock() is used to protect access,
 // reference counts are used to ensure that individual
 // LogTimeEntry lifetime is managed when not protected.
 void FlushCommand::runSocketCommand(SocketClient* client) {
     LogTimeEntry* entry = NULL;
     LastLogTimes& times = mReader.logbuf().mTimes;
 
-    LogTimeEntry::lock();
+    LogTimeEntry::wrlock();
     LastLogTimes::iterator it = times.begin();
     while (it != times.end()) {
         entry = (*it);

logd/LogAudit.cpp

@@ -277,7 +277,7 @@ int LogAudit::logPrint(const char* fmt, ...) {
             ++cp;
         }
         tid = pid;
-        logbuf->lock();
+        logbuf->wrlock();
         uid = logbuf->pidToUid(pid);
         logbuf->unlock();
         memmove(pidptr, cp, strlen(cp) + 1);
@@ -322,7 +322,7 @@ int LogAudit::logPrint(const char* fmt, ...) {
         pid = tid;
         comm = "auditd";
     } else {
-        logbuf->lock();
+        logbuf->wrlock();
         comm = commfree = logbuf->pidToName(pid);
         logbuf->unlock();
         if (!comm) {

logd/LogBuffer.cpp

@@ -72,7 +72,7 @@ void LogBuffer::init() {
         // as the act of mounting /data would trigger persist.logd.timestamp to
         // be corrected. 1/30 corner case YMMV.
         //
-        pthread_mutex_lock(&mLogElementsLock);
+        rdlock();
         LogBufferElementCollection::iterator it = mLogElements.begin();
         while ((it != mLogElements.end())) {
             LogBufferElement* e = *it;
@@ -87,15 +87,15 @@ void LogBuffer::init() {
             }
             ++it;
         }
-        pthread_mutex_unlock(&mLogElementsLock);
+        unlock();
     }
 
     // We may have been triggered by a SIGHUP. Release any sleeping reader
     // threads to dump their current content.
     //
     // NB: this is _not_ performed in the context of a SIGHUP, it is
     // performed during startup, and in context of reinit administrative thread
-    LogTimeEntry::lock();
+    LogTimeEntry::wrlock();
 
     LastLogTimes::iterator times = mTimes.begin();
     while (times != mTimes.end()) {
@@ -111,7 +111,7 @@ void LogBuffer::init() {
 
 LogBuffer::LogBuffer(LastLogTimes* times)
     : monotonic(android_log_clockid() == CLOCK_MONOTONIC), mTimes(*times) {
-    pthread_mutex_init(&mLogElementsLock, nullptr);
+    pthread_rwlock_init(&mLogElementsLock, nullptr);
 
     log_id_for_each(i) {
         lastLoggedElements[i] = nullptr;
@@ -207,15 +207,15 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,
         }
         if (!__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE)) {
             // Log traffic received to total
-            pthread_mutex_lock(&mLogElementsLock);
+            wrlock();
             stats.addTotal(elem);
-            pthread_mutex_unlock(&mLogElementsLock);
+            unlock();
             delete elem;
             return -EACCES;
         }
     }
 
-    pthread_mutex_lock(&mLogElementsLock);
+    wrlock();
     LogBufferElement* currentLast = lastLoggedElements[log_id];
     if (currentLast) {
         LogBufferElement* dropped = droppedElements[log_id];
@@ -316,14 +316,14 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,
                     // check for overflow
                     if (total >= UINT32_MAX) {
                         log(currentLast);
-                        pthread_mutex_unlock(&mLogElementsLock);
+                        unlock();
                         return len;
                     }
                     stats.addTotal(currentLast);
                     delete currentLast;
                     swab = total;
                     event->payload.data = htole32(swab);
-                    pthread_mutex_unlock(&mLogElementsLock);
+                    unlock();
                     return len;
                 }
                 if (count == USHRT_MAX) {
@@ -340,7 +340,7 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,
             }
             droppedElements[log_id] = currentLast;
             lastLoggedElements[log_id] = elem;
-            pthread_mutex_unlock(&mLogElementsLock);
+            unlock();
             return len;
         }
         if (dropped) {         // State 1 or 2
@@ -358,12 +358,12 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,
     lastLoggedElements[log_id] = new LogBufferElement(*elem);
 
     log(elem);
-    pthread_mutex_unlock(&mLogElementsLock);
+    unlock();
 
     return len;
 }
 
-// assumes mLogElementsLock held, owns elem, will look after garbage collection
+// assumes LogBuffer::wrlock() held, owns elem, look after garbage collection
 void LogBuffer::log(LogBufferElement* elem) {
     // cap on how far back we will sort in-place, otherwise append
     static uint32_t too_far_back = 5;  // five seconds
@@ -384,7 +384,7 @@ void LogBuffer::log(LogBufferElement* elem) {
         bool end_set = false;
         bool end_always = false;
 
-        LogTimeEntry::lock();
+        LogTimeEntry::rdlock();
 
         LastLogTimes::iterator times = mTimes.begin();
         while (times != mTimes.end()) {
@@ -426,7 +426,7 @@ void LogBuffer::log(LogBufferElement* elem) {
 
 // Prune at most 10% of the log entries or maxPrune, whichever is less.
 //
-// mLogElementsLock must be held when this function is called.
+// LogBuffer::wrlock() must be held when this function is called.
 void LogBuffer::maybePrune(log_id_t id) {
     size_t sizes = stats.sizes(id);
     unsigned long maxSize = log_buffer_size(id);
@@ -650,14 +650,14 @@ class LogBufferElementLast {
 // The third thread is optional, and only gets hit if there was a whitelist
 // and more needs to be pruned against the backstop of the region lock.
 //
-// mLogElementsLock must be held when this function is called.
+// LogBuffer::wrlock() must be held when this function is called.
 //
 bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) {
     LogTimeEntry* oldest = nullptr;
     bool busy = false;
     bool clearAll = pruneRows == ULONG_MAX;
 
-    LogTimeEntry::lock();
+    LogTimeEntry::rdlock();
 
     // Region locked?
     LastLogTimes::iterator times = mTimes.begin();
@@ -1017,15 +1017,15 @@ bool LogBuffer::clear(log_id_t id, uid_t uid) {
             // one entry, not another clear run, so we are looking for
             // the quick side effect of the return value to tell us if
             // we have a _blocked_ reader.
-            pthread_mutex_lock(&mLogElementsLock);
+            wrlock();
             busy = prune(id, 1, uid);
-            pthread_mutex_unlock(&mLogElementsLock);
+            unlock();
             // It is still busy, blocked reader(s), lets kill them all!
             // otherwise, lets be a good citizen and preserve the slow
             // readers and let the clear run (below) deal with determining
             // if we are still blocked and return an error code to caller.
             if (busy) {
-                LogTimeEntry::lock();
+                LogTimeEntry::wrlock();
                 LastLogTimes::iterator times = mTimes.begin();
                 while (times != mTimes.end()) {
                     LogTimeEntry* entry = (*times);
@@ -1038,9 +1038,9 @@ bool LogBuffer::clear(log_id_t id, uid_t uid) {
                 LogTimeEntry::unlock();
             }
         }
-        pthread_mutex_lock(&mLogElementsLock);
+        wrlock();
         busy = prune(id, ULONG_MAX, uid);
-        pthread_mutex_unlock(&mLogElementsLock);
+        unlock();
         if (!busy || !--retry) {
             break;
         }
@@ -1051,9 +1051,9 @@ bool LogBuffer::clear(log_id_t id, uid_t uid) {
 
 // get the used space associated with "id".
 unsigned long LogBuffer::getSizeUsed(log_id_t id) {
-    pthread_mutex_lock(&mLogElementsLock);
+    rdlock();
     size_t retval = stats.sizes(id);
-    pthread_mutex_unlock(&mLogElementsLock);
+    unlock();
     return retval;
 }
 
@@ -1063,17 +1063,17 @@ int LogBuffer::setSize(log_id_t id, unsigned long size) {
     if (!__android_logger_valid_buffer_size(size)) {
         return -1;
     }
-    pthread_mutex_lock(&mLogElementsLock);
+    wrlock();
     log_buffer_size(id) = size;
-    pthread_mutex_unlock(&mLogElementsLock);
+    unlock();
     return 0;
 }
 
 // get the total space allocated to "id"
 unsigned long LogBuffer::getSize(log_id_t id) {
-    pthread_mutex_lock(&mLogElementsLock);
+    rdlock();
     size_t retval = log_buffer_size(id);
-    pthread_mutex_unlock(&mLogElementsLock);
+    unlock();
     return retval;
 }
 
@@ -1085,7 +1085,7 @@ log_time LogBuffer::flushTo(SocketClient* reader, const log_time& start,
     LogBufferElementCollection::iterator it;
     uid_t uid = reader->getUid();
 
-    pthread_mutex_lock(&mLogElementsLock);
+    rdlock();
 
     if (start == log_time::EPOCH) {
         // client wants to start from the beginning
@@ -1143,7 +1143,7 @@ log_time LogBuffer::flushTo(SocketClient* reader, const log_time& start,
             continue;
         }
 
-        // NB: calling out to another object with mLogElementsLock held (safe)
+        // NB: calling out to another object with wrlock() held (safe)
         if (filter) {
             int ret = (*filter)(element, arg);
             if (ret == false) {
@@ -1166,7 +1166,7 @@ log_time LogBuffer::flushTo(SocketClient* reader, const log_time& start,
                 (element->getDropped() && !sameTid) ? 0 : element->getTid();
         }
 
-        pthread_mutex_unlock(&mLogElementsLock);
+        unlock();
 
         // range locking in LastLogTimes looks after us
         max = element->flushTo(reader, this, privileged, sameTid);
@@ -1176,20 +1176,20 @@ log_time LogBuffer::flushTo(SocketClient* reader, const log_time& start,
         }
 
         skip = maxSkip;
-        pthread_mutex_lock(&mLogElementsLock);
+        rdlock();
     }
-    pthread_mutex_unlock(&mLogElementsLock);
+    unlock();
 
     return max;
 }
 
 std::string LogBuffer::formatStatistics(uid_t uid, pid_t pid,
                                         unsigned int logMask) {
-    pthread_mutex_lock(&mLogElementsLock);
+    wrlock();
 
     std::string ret = stats.format(uid, pid, logMask);
 
-    pthread_mutex_unlock(&mLogElementsLock);
+    unlock();
 
     return ret;
 }

logd/LogBuffer.h

@@ -76,7 +76,7 @@ typedef std::list<LogBufferElement*> LogBufferElementCollection;
 
 class LogBuffer {
     LogBufferElementCollection mLogElements;
-    pthread_mutex_t mLogElementsLock;
+    pthread_rwlock_t mLogElementsLock;
 
     LogStatistics stats;
 
@@ -154,7 +154,7 @@ class LogBuffer {
         return tags.tagToName(tag);
     }
 
-    // helper must be protected directly or implicitly by lock()/unlock()
+    // helper must be protected directly or implicitly by wrlock()/unlock()
     const char* pidToName(pid_t pid) {
         return stats.pidToName(pid);
     }
@@ -164,11 +164,14 @@ class LogBuffer {
     const char* uidToName(uid_t uid) {
         return stats.uidToName(uid);
     }
-    void lock() {
-        pthread_mutex_lock(&mLogElementsLock);
+    void wrlock() {
+        pthread_rwlock_wrlock(&mLogElementsLock);
+    }
+    void rdlock() {
+        pthread_rwlock_rdlock(&mLogElementsLock);
     }
     void unlock() {
-        pthread_mutex_unlock(&mLogElementsLock);
+        pthread_rwlock_unlock(&mLogElementsLock);
     }
 
    private:

logd/LogBufferElement.cpp

@@ -121,15 +121,15 @@ size_t LogBufferElement::populateDroppedMessage(char*& buffer, LogBuffer* parent
     }
 
     static const char format_uid[] = "uid=%u%s%s %s %u line%s";
-    parent->lock();
+    parent->wrlock();
     const char* name = parent->uidToName(mUid);
     parent->unlock();
     const char* commName = android::tidToName(mTid);
     if (!commName && (mTid != mPid)) {
         commName = android::tidToName(mPid);
     }
     if (!commName) {
-        parent->lock();
+        parent->wrlock();
         commName = parent->pidToName(mPid);
         parent->unlock();
     }

logd/LogKlog.cpp

@@ -579,7 +579,7 @@ int LogKlog::log(const char* buf, ssize_t len) {
     const pid_t tid = pid;
     uid_t uid = AID_ROOT;
     if (pid) {
-        logbuf->lock();
+        logbuf->wrlock();
         uid = logbuf->pidToUid(pid);
         logbuf->unlock();
     }

logd/LogReader.cpp

@@ -111,7 +111,7 @@ bool LogReader::onDataAvailable(SocketClient* cli) {
     if (!fastcmp<strncmp>(buffer, "dumpAndClose", 12)) {
         // Allow writer to get some cycles, and wait for pending notifications
         sched_yield();
-        LogTimeEntry::lock();
+        LogTimeEntry::wrlock();
         LogTimeEntry::unlock();
         sched_yield();
         nonBlock = true;
@@ -212,7 +212,7 @@ bool LogReader::onDataAvailable(SocketClient* cli) {
 
 void LogReader::doSocketDelete(SocketClient* cli) {
     LastLogTimes& times = mLogbuf.mTimes;
-    LogTimeEntry::lock();
+    LogTimeEntry::wrlock();
     LastLogTimes::iterator it = times.begin();
     while (it != times.end()) {
         LogTimeEntry* entry = (*it);

logd/LogStatistics.cpp

@@ -228,6 +228,7 @@ void LogStatistics::drop(LogBufferElement* element) {
 }
 
 // caller must own and free character string
+// Requires parent LogBuffer::wrlock() to be held
 const char* LogStatistics::uidToName(uid_t uid) const {
     // Local hard coded favourites
     if (uid == AID_LOGD) {