From 982f7bd3459e8b260091eba27051ec62bc00b09d Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 12 Feb 2019 13:59:03 -0800 Subject: [PATCH] base: add ScopedLockAssertion. This is a useful helper for anyone that's using thread safety annotations alongside std::condition_variable, extract it from adb and move it to libbase. Test: mma Change-Id: Ic51e2f2a0e6a16628034b29d8ff32bf2155afccd --- adb/transport.cpp | 19 +++------- .../include/android-base/thread_annotations.h | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/adb/transport.cpp b/adb/transport.cpp index ae5359794..90f94ee1e 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -52,6 +52,8 @@ #include "fdevent.h" #include "sysdeps/chrono.h" +using android::base::ScopedLockAssertion; + static void remove_transport(atransport* transport); static void transport_unref(atransport* transport); @@ -72,17 +74,6 @@ const char* const kFeatureAbb = "abb"; namespace { -// A class that helps the Clang Thread Safety Analysis deal with -// std::unique_lock. Given that std::unique_lock is movable, and the analysis -// can not currently perform alias analysis, it is not annotated. In order to -// assert that the mutex is held, a ScopedAssumeLocked can be created just after -// the std::unique_lock. -class SCOPED_CAPABILITY ScopedAssumeLocked { - public: - ScopedAssumeLocked(std::mutex& mutex) ACQUIRE(mutex) {} - ~ScopedAssumeLocked() RELEASE() {} -}; - #if ADB_HOST // Tracks and handles atransport*s that are attempting reconnection. class ReconnectHandler { @@ -180,7 +171,7 @@ void ReconnectHandler::Run() { ReconnectAttempt attempt; { std::unique_lock lock(reconnect_mutex_); - ScopedAssumeLocked assume_lock(reconnect_mutex_); + ScopedLockAssertion assume_lock(reconnect_mutex_); if (!reconnect_queue_.empty()) { // FIXME: libstdc++ (used on Windows) implements condition_variable with @@ -296,7 +287,7 @@ void BlockingConnectionAdapter::Start() { LOG(INFO) << this->transport_name_ << ": write thread spawning"; while (true) { std::unique_lock lock(mutex_); - ScopedAssumeLocked assume_locked(mutex_); + ScopedLockAssertion assume_locked(mutex_); cv_.wait(lock, [this]() REQUIRES(mutex_) { return this->stopped_ || !this->write_queue_.empty(); }); @@ -923,7 +914,7 @@ atransport* acquire_one_transport(TransportType type, const char* serial, Transp bool ConnectionWaitable::WaitForConnection(std::chrono::milliseconds timeout) { std::unique_lock lock(mutex_); - ScopedAssumeLocked assume_locked(mutex_); + ScopedLockAssertion assume_locked(mutex_); return cv_.wait_for(lock, timeout, [&]() REQUIRES(mutex_) { return connection_established_ready_; }) && connection_established_; diff --git a/base/include/android-base/thread_annotations.h b/base/include/android-base/thread_annotations.h index 5c55e6300..53fe6dae9 100644 --- a/base/include/android-base/thread_annotations.h +++ b/base/include/android-base/thread_annotations.h @@ -16,6 +16,8 @@ #pragma once +#include + #define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x)) #define CAPABILITY(x) \ @@ -104,3 +106,39 @@ #define NO_THREAD_SAFETY_ANALYSIS \ THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis) + +namespace android { +namespace base { + +// A class to help thread safety analysis deal with std::unique_lock and condition_variable. +// +// Clang's thread safety analysis currently doesn't perform alias analysis, so movable types +// like std::unique_lock can't be marked with thread safety annotations. This helper allows +// for manual assertion of lock state in a scope. +// +// For example: +// +// std::mutex mutex; +// std::condition_variable cv; +// std::vector vec GUARDED_BY(mutex); +// +// int pop() { +// std::unique_lock lock(mutex); +// ScopedLockAssertion lock_assertion(mutex); +// cv.wait(lock, []() { +// ScopedLockAssertion lock_assertion(mutex); +// return !vec.empty(); +// }); +// +// int result = vec.back(); +// vec.pop_back(); +// return result; +// } +class SCOPED_CAPABILITY ScopedLockAssertion { + public: + ScopedLockAssertion(std::mutex& mutex) ACQUIRE(mutex) {} + ~ScopedLockAssertion() RELEASE() {} +}; + +} // namespace base +} // namespace android