logd: use RAII locks and thread annotations
Test: unit tests
Change-Id: I38623130a96f17a47ed79753e24b25efa9e38279
diff --git a/logd/ChattyLogBuffer.cpp b/logd/ChattyLogBuffer.cpp
index f1305a5..369587a 100644
--- a/logd/ChattyLogBuffer.cpp
+++ b/logd/ChattyLogBuffer.cpp
@@ -59,8 +59,6 @@
ChattyLogBuffer::ChattyLogBuffer(LogReaderList* reader_list, LogTags* tags, PruneList* prune,
LogStatistics* stats)
: reader_list_(reader_list), tags_(tags), prune_(prune), stats_(stats) {
- pthread_rwlock_init(&mLogElementsLock, nullptr);
-
log_id_for_each(i) {
lastLoggedElements[i] = nullptr;
droppedElements[i] = nullptr;
@@ -162,10 +160,8 @@
// b/137093665: don't coalesce security messages.
if (log_id == LOG_ID_SECURITY) {
- wrlock();
+ auto lock = std::lock_guard{lock_};
log(elem);
- unlock();
-
return len;
}
@@ -189,7 +185,7 @@
return -EACCES;
}
- wrlock();
+ auto lock = std::lock_guard{lock_};
LogBufferElement* currentLast = lastLoggedElements[log_id];
if (currentLast) {
LogBufferElement* dropped = droppedElements[log_id];
@@ -289,14 +285,12 @@
// check for overflow
if (total >= std::numeric_limits<int32_t>::max()) {
log(currentLast);
- unlock();
return len;
}
stats_->AddTotal(currentLast);
delete currentLast;
swab = total;
event->payload.data = htole32(swab);
- unlock();
return len;
}
if (count == USHRT_MAX) {
@@ -313,7 +307,6 @@
}
droppedElements[log_id] = currentLast;
lastLoggedElements[log_id] = elem;
- unlock();
return len;
}
if (dropped) { // State 1 or 2
@@ -331,12 +324,9 @@
lastLoggedElements[log_id] = new LogBufferElement(*elem);
log(elem);
- unlock();
-
return len;
}
-// assumes ChattyLogBuffer::wrlock() held, owns elem, look after garbage collection
void ChattyLogBuffer::log(LogBufferElement* elem) {
mLogElements.push_back(elem);
stats_->Add(elem);
@@ -344,7 +334,6 @@
reader_list_->NotifyNewLog(1 << elem->getLogId());
}
-// ChattyLogBuffer::wrlock() must be held when this function is called.
void ChattyLogBuffer::maybePrune(log_id_t id) {
unsigned long prune_rows;
if (stats_->ShouldPrune(id, log_buffer_size(id), &prune_rows)) {
@@ -540,8 +529,6 @@
// 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.
//
-// ChattyLogBuffer::wrlock() must be held when this function is called.
-//
bool ChattyLogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) {
LogReaderThread* oldest = nullptr;
bool busy = false;
@@ -846,9 +833,10 @@
// 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.
- wrlock();
- busy = prune(id, 1, uid);
- unlock();
+ {
+ auto lock = std::lock_guard{lock_};
+ busy = prune(id, 1, uid);
+ }
// 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
@@ -865,9 +853,10 @@
}
}
}
- wrlock();
- busy = prune(id, ULONG_MAX, uid);
- unlock();
+ {
+ auto lock = std::lock_guard{lock_};
+ busy = prune(id, ULONG_MAX, uid);
+ }
if (!busy || !--retry) {
break;
}
@@ -882,17 +871,15 @@
if (!__android_logger_valid_buffer_size(size)) {
return -1;
}
- wrlock();
+ auto lock = std::lock_guard{lock_};
log_buffer_size(id) = size;
- unlock();
return 0;
}
// get the total space allocated to "id"
unsigned long ChattyLogBuffer::GetSize(log_id_t id) {
- rdlock();
+ auto shared_lock = SharedLock{lock_};
size_t retval = log_buffer_size(id);
- unlock();
return retval;
}
@@ -902,7 +889,7 @@
LogBufferElementCollection::iterator it;
uid_t uid = writer->uid();
- rdlock();
+ auto shared_lock = SharedLock{lock_};
if (start <= 1) {
// client wants to start from the beginning
@@ -957,7 +944,7 @@
(element->getDropped() && !sameTid) ? 0 : element->getTid();
}
- unlock();
+ shared_lock.unlock();
curr = element->getSequence();
// range locking in LastLogTimes looks after us
@@ -965,9 +952,7 @@
return FLUSH_ERROR;
}
- rdlock();
+ shared_lock.lock_shared();
}
- unlock();
-
return curr;
}
diff --git a/logd/ChattyLogBuffer.h b/logd/ChattyLogBuffer.h
index 29a421d..42cdeec 100644
--- a/logd/ChattyLogBuffer.h
+++ b/logd/ChattyLogBuffer.h
@@ -22,6 +22,7 @@
#include <optional>
#include <string>
+#include <android-base/thread_annotations.h>
#include <android/log.h>
#include <private/android_filesystem_config.h>
#include <sysutils/SocketClient.h>
@@ -34,25 +35,25 @@
#include "LogTags.h"
#include "LogWhiteBlackList.h"
#include "LogWriter.h"
+#include "rwlock.h"
typedef std::list<LogBufferElement*> LogBufferElementCollection;
class ChattyLogBuffer : public LogBuffer {
- LogBufferElementCollection mLogElements;
- pthread_rwlock_t mLogElementsLock;
+ LogBufferElementCollection mLogElements GUARDED_BY(lock_);
// watermark of any worst/chatty uid processing
typedef std::unordered_map<uid_t, LogBufferElementCollection::iterator> LogBufferIteratorMap;
- LogBufferIteratorMap mLastWorst[LOG_ID_MAX];
+ LogBufferIteratorMap mLastWorst[LOG_ID_MAX] GUARDED_BY(lock_);
// watermark of any worst/chatty pid of system processing
typedef std::unordered_map<pid_t, LogBufferElementCollection::iterator> LogBufferPidIteratorMap;
- LogBufferPidIteratorMap mLastWorstPidOfSystem[LOG_ID_MAX];
+ LogBufferPidIteratorMap mLastWorstPidOfSystem[LOG_ID_MAX] GUARDED_BY(lock_);
- unsigned long mMaxSize[LOG_ID_MAX];
+ unsigned long mMaxSize[LOG_ID_MAX] GUARDED_BY(lock_);
- LogBufferElement* lastLoggedElements[LOG_ID_MAX];
- LogBufferElement* droppedElements[LOG_ID_MAX];
- void log(LogBufferElement* elem);
+ LogBufferElement* lastLoggedElements[LOG_ID_MAX] GUARDED_BY(lock_);
+ LogBufferElement* droppedElements[LOG_ID_MAX] GUARDED_BY(lock_);
+ void log(LogBufferElement* elem) REQUIRES(lock_);
public:
ChattyLogBuffer(LogReaderList* reader_list, LogTags* tags, PruneList* prune,
@@ -71,20 +72,16 @@
int SetSize(log_id_t id, unsigned long size) override;
private:
- void wrlock() { pthread_rwlock_wrlock(&mLogElementsLock); }
- void rdlock() { pthread_rwlock_rdlock(&mLogElementsLock); }
- void unlock() { pthread_rwlock_unlock(&mLogElementsLock); }
+ void maybePrune(log_id_t id) REQUIRES(lock_);
+ void kickMe(LogReaderThread* me, log_id_t id, unsigned long pruneRows) REQUIRES_SHARED(lock_);
- void maybePrune(log_id_t id);
- void kickMe(LogReaderThread* me, log_id_t id, unsigned long pruneRows);
-
- bool prune(log_id_t id, unsigned long pruneRows, uid_t uid = AID_ROOT);
+ bool prune(log_id_t id, unsigned long pruneRows, uid_t uid = AID_ROOT) REQUIRES(lock_);
LogBufferElementCollection::iterator erase(LogBufferElementCollection::iterator it,
- bool coalesce = false);
+ bool coalesce = false) REQUIRES(lock_);
// Returns an iterator to the oldest element for a given log type, or mLogElements.end() if
// there are no logs for the given log type. Requires mLogElementsLock to be held.
- LogBufferElementCollection::iterator GetOldest(log_id_t log_id);
+ LogBufferElementCollection::iterator GetOldest(log_id_t log_id) REQUIRES(lock_);
LogReaderList* reader_list_;
LogTags* tags_;
@@ -94,4 +91,6 @@
// Keeps track of the iterator to the oldest log message of a given log type, as an
// optimization when pruning logs. Use GetOldest() to retrieve.
std::optional<LogBufferElementCollection::iterator> oldest_[LOG_ID_MAX];
+
+ RwLock lock_;
};
diff --git a/logd/rwlock.h b/logd/rwlock.h
new file mode 100644
index 0000000..2b27ff1
--- /dev/null
+++ b/logd/rwlock.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <pthread.h>
+
+#include <android-base/macros.h>
+#include <android-base/thread_annotations.h>
+
+// As of the end of May 2020, std::shared_mutex is *not* simply a pthread_rwlock, but rather a
+// combination of std::mutex and std::condition variable, which is obviously less efficient. This
+// immitates what std::shared_mutex should be doing and is compatible with RAII thread wrappers.
+
+class SHARED_CAPABILITY("mutex") RwLock {
+ public:
+ RwLock() {}
+ ~RwLock() {}
+
+ void lock() ACQUIRE() { pthread_rwlock_wrlock(&rwlock_); }
+ void lock_shared() ACQUIRE_SHARED() { pthread_rwlock_rdlock(&rwlock_); }
+
+ void unlock() RELEASE() { pthread_rwlock_unlock(&rwlock_); }
+
+ private:
+ pthread_rwlock_t rwlock_ = PTHREAD_RWLOCK_INITIALIZER;
+};
+
+// std::shared_lock does not have thread annotations, so we need our own.
+
+class SCOPED_CAPABILITY SharedLock {
+ public:
+ SharedLock(RwLock& lock) ACQUIRE_SHARED(lock) : lock_(lock) { lock_.lock_shared(); }
+ ~SharedLock() RELEASE() { lock_.unlock(); }
+
+ void lock_shared() ACQUIRE_SHARED() { lock_.lock_shared(); }
+ void unlock() RELEASE() { lock_.unlock(); }
+
+ DISALLOW_IMPLICIT_CONSTRUCTORS(SharedLock);
+
+ private:
+ RwLock& lock_;
+};