From 66790ccfcb0ad3fad7c4454ab590cc10e1e6863f Mon Sep 17 00:00:00 2001 From: yanavlasov Date: Fri, 13 Sep 2019 17:37:33 -0400 Subject: [PATCH] Replace deprecated thread annotations macros. (#8237) Abseil thread annotation macros are now prefixed by ABSL_. There is no semantic change; this is just a rename. Signed-off-by: Yan Avlasov --- include/envoy/thread/thread.h | 8 +++---- .../access_log/access_log_manager_impl.h | 12 +++++----- source/common/common/lock_guard.h | 23 ++++++++++--------- source/common/common/logger.h | 2 +- source/common/common/thread.h | 14 +++++------ source/common/event/dispatcher_impl.h | 2 +- source/common/http/codes.h | 2 +- source/common/runtime/runtime_impl.h | 2 +- source/common/upstream/thread_aware_lb_impl.h | 6 ++--- source/common/upstream/upstream_impl.h | 2 +- .../transport_sockets/tls/context_impl.h | 2 +- .../transport_sockets/tls/ssl_socket.h | 4 ++-- source/server/guarddog_impl.h | 4 ++-- test/common/common/lock_guard_test.cc | 2 +- test/integration/fake_upstream.h | 14 +++++------ test/integration/filters/pause_filter.cc | 7 +++--- .../filters/random_pause_filter.cc | 2 +- test/mocks/common.h | 2 +- test/server/guarddog_impl_test.cc | 2 +- 19 files changed, 57 insertions(+), 55 deletions(-) diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h index 8d630d8ac134..70452ca5d29a 100644 --- a/include/envoy/thread/thread.h +++ b/include/envoy/thread/thread.h @@ -67,13 +67,13 @@ class ThreadFactory { * Like the C++11 "basic lockable concept" but a pure virtual interface vs. a template, and * with thread annotations. */ -class LOCKABLE BasicLockable { +class ABSL_LOCKABLE BasicLockable { public: virtual ~BasicLockable() = default; - virtual void lock() EXCLUSIVE_LOCK_FUNCTION() PURE; - virtual bool tryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true) PURE; - virtual void unlock() UNLOCK_FUNCTION() PURE; + virtual void lock() ABSL_EXCLUSIVE_LOCK_FUNCTION() PURE; + virtual bool tryLock() ABSL_EXCLUSIVE_TRYLOCK_FUNCTION(true) PURE; + virtual void unlock() ABSL_UNLOCK_FUNCTION() PURE; }; } // namespace Thread diff --git a/source/common/access_log/access_log_manager_impl.h b/source/common/access_log/access_log_manager_impl.h index 64c9c2594b49..c145be337101 100644 --- a/source/common/access_log/access_log_manager_impl.h +++ b/source/common/access_log/access_log_manager_impl.h @@ -114,12 +114,12 @@ class AccessLogFileImpl : public AccessLogFile { std::atomic flush_thread_exit_{}; std::atomic reopen_file_{}; Buffer::OwnedImpl - flush_buffer_ GUARDED_BY(write_lock_); // This buffer is used by multiple threads. It gets - // filled and then flushed either when max size is - // reached or when a timer fires. - // TODO(jmarantz): this should be GUARDED_BY(flush_lock_) but the analysis cannot poke through - // the std::make_unique assignment. I do not believe it's possible to annotate this properly now - // due to limitations in the clang thread annotation analysis. + flush_buffer_ ABSL_GUARDED_BY(write_lock_); // This buffer is used by multiple threads. It + // gets filled and then flushed either when max + // size is reached or when a timer fires. + // TODO(jmarantz): this should be ABSL_GUARDED_BY(flush_lock_) but the analysis cannot poke + // through the std::make_unique assignment. I do not believe it's possible to annotate this + // properly now due to limitations in the clang thread annotation analysis. Buffer::OwnedImpl about_to_write_buffer_; // This buffer is used only by the flush thread. Data // is moved from flush_buffer_ under lock, and then // the lock is released so that flush_buffer_ can diff --git a/source/common/common/lock_guard.h b/source/common/common/lock_guard.h index cde6da486ee5..d3025a333999 100644 --- a/source/common/common/lock_guard.h +++ b/source/common/common/lock_guard.h @@ -11,14 +11,14 @@ namespace Thread { /** * A lock guard that deals with an optional lock. */ -class SCOPED_LOCKABLE OptionalLockGuard { +class ABSL_SCOPED_LOCKABLE OptionalLockGuard { public: /** * Establishes a scoped mutex-lock. If non-null, the mutex is locked upon construction. * * @param lock the mutex. */ - OptionalLockGuard(BasicLockable* lock) EXCLUSIVE_LOCK_FUNCTION(lock) : lock_(lock) { + OptionalLockGuard(BasicLockable* lock) ABSL_EXCLUSIVE_LOCK_FUNCTION(lock) : lock_(lock) { if (lock_ != nullptr) { lock_->lock(); } @@ -27,7 +27,7 @@ class SCOPED_LOCKABLE OptionalLockGuard { /** * Destruction of the OptionalLockGuard unlocks the lock, if it is non-null. */ - ~OptionalLockGuard() UNLOCK_FUNCTION() { + ~OptionalLockGuard() ABSL_UNLOCK_FUNCTION() { if (lock_ != nullptr) { lock_->unlock(); } @@ -50,7 +50,7 @@ class SCOPED_LOCKABLE OptionalLockGuard { * class lacks thread annotations, as clang currently does appear to be able to handle * conditional thread annotations. So the ones we'd like are commented out. */ -class SCOPED_LOCKABLE TryLockGuard { +class ABSL_SCOPED_LOCKABLE TryLockGuard { public: /** * Establishes a scoped mutex-lock; the a mutex lock is attempted via tryLock, so @@ -92,21 +92,21 @@ class SCOPED_LOCKABLE TryLockGuard { * implementation (no conditionals) and readability at call-sites. In some cases, an early * release is needed, in which case, a ReleasableLockGuard can be used. */ -class SCOPED_LOCKABLE LockGuard { +class ABSL_SCOPED_LOCKABLE LockGuard { public: /** * Establishes a scoped mutex-lock; the mutex is locked upon construction. * * @param lock the mutex. */ - explicit LockGuard(BasicLockable& lock) EXCLUSIVE_LOCK_FUNCTION(lock) : lock_(lock) { + explicit LockGuard(BasicLockable& lock) ABSL_EXCLUSIVE_LOCK_FUNCTION(lock) : lock_(lock) { lock_.lock(); } /** * Destruction of the LockGuard unlocks the lock. */ - ~LockGuard() UNLOCK_FUNCTION() { lock_.unlock(); } + ~LockGuard() ABSL_UNLOCK_FUNCTION() { lock_.unlock(); } private: BasicLockable& lock_; @@ -117,27 +117,28 @@ class SCOPED_LOCKABLE LockGuard { * BasicLockable& to allow usages to be agnostic to cross-process mutexes vs. single-process * mutexes. */ -class SCOPED_LOCKABLE ReleasableLockGuard { +class ABSL_SCOPED_LOCKABLE ReleasableLockGuard { public: /** * Establishes a scoped mutex-lock; the mutex is locked upon construction. * * @param lock the mutex. */ - explicit ReleasableLockGuard(BasicLockable& lock) EXCLUSIVE_LOCK_FUNCTION(lock) : lock_(&lock) { + explicit ReleasableLockGuard(BasicLockable& lock) ABSL_EXCLUSIVE_LOCK_FUNCTION(lock) + : lock_(&lock) { lock_->lock(); } /** * Destruction of the LockGuard unlocks the lock, if it has not already been explicitly released. */ - ~ReleasableLockGuard() UNLOCK_FUNCTION() { release(); } + ~ReleasableLockGuard() ABSL_UNLOCK_FUNCTION() { release(); } /** * Unlocks the mutex. This enables call-sites to release the mutex prior to the Lock going out of * scope. This is called release() for consistency with absl::ReleasableMutexLock. */ - void release() UNLOCK_FUNCTION() { + void release() ABSL_UNLOCK_FUNCTION() { if (lock_ != nullptr) { lock_->unlock(); lock_ = nullptr; diff --git a/source/common/common/logger.h b/source/common/common/logger.h index a443ce11ab5e..22a0255a0e9f 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -189,7 +189,7 @@ class DelegatingLogSink : public spdlog::sinks::sink { SinkDelegate* sink_{nullptr}; std::unique_ptr stderr_sink_; // Builtin sink to use as a last resort. - std::unique_ptr formatter_ GUARDED_BY(format_mutex_); + std::unique_ptr formatter_ ABSL_GUARDED_BY(format_mutex_); absl::Mutex format_mutex_; // direct absl reference to break build cycle. }; diff --git a/source/common/common/thread.h b/source/common/common/thread.h index 071699fc49a4..f447ecfb83db 100644 --- a/source/common/common/thread.h +++ b/source/common/common/thread.h @@ -16,9 +16,9 @@ namespace Thread { class MutexBasicLockable : public BasicLockable { public: // BasicLockable - void lock() EXCLUSIVE_LOCK_FUNCTION() override { mutex_.Lock(); } - bool tryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true) override { return mutex_.TryLock(); } - void unlock() UNLOCK_FUNCTION() override { mutex_.Unlock(); } + void lock() ABSL_EXCLUSIVE_LOCK_FUNCTION() override { mutex_.Lock(); } + bool tryLock() ABSL_EXCLUSIVE_TRYLOCK_FUNCTION(true) override { return mutex_.TryLock(); } + void unlock() ABSL_UNLOCK_FUNCTION() override { mutex_.Unlock(); } private: friend class CondVar; @@ -52,7 +52,7 @@ class CondVar { * source/source/thread.h for an alternate implementation, which does not work * with thread annotation. */ - void wait(MutexBasicLockable& mutex) noexcept EXCLUSIVE_LOCKS_REQUIRED(mutex) { + void wait(MutexBasicLockable& mutex) noexcept ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex) { condvar_.Wait(&mutex.mutex_); } template @@ -60,9 +60,9 @@ class CondVar { /** * @return WaitStatus whether the condition timed out or not. */ - WaitStatus - waitFor(MutexBasicLockable& mutex, - std::chrono::duration duration) noexcept EXCLUSIVE_LOCKS_REQUIRED(mutex) { + WaitStatus waitFor( + MutexBasicLockable& mutex, + std::chrono::duration duration) noexcept ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex) { return condvar_.WaitWithTimeout(&mutex.mutex_, absl::FromChrono(duration)) ? WaitStatus::Timeout : WaitStatus::NoTimeout; diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 5cbccddb66e0..8108b3249dc0 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -110,7 +110,7 @@ class DispatcherImpl : Logger::Loggable, std::vector to_delete_2_; std::vector* current_to_delete_; Thread::MutexBasicLockable post_lock_; - std::list> post_callbacks_ GUARDED_BY(post_lock_); + std::list> post_callbacks_ ABSL_GUARDED_BY(post_lock_); const ScopeTrackedObject* current_object_{}; bool deferred_deleting_{}; }; diff --git a/source/common/http/codes.h b/source/common/http/codes.h index 45d51a29ff91..c312f167597d 100644 --- a/source/common/http/codes.h +++ b/source/common/http/codes.h @@ -62,7 +62,7 @@ class CodeStatsImpl : public CodeStats { Stats::StatName upstreamRqGroup(Code response_code) const; Stats::StatName upstreamRqStatName(Code response_code) const; - mutable Stats::StatNamePool stat_name_pool_ GUARDED_BY(mutex_); + mutable Stats::StatNamePool stat_name_pool_ ABSL_GUARDED_BY(mutex_); mutable absl::Mutex mutex_; Stats::SymbolTable& symbol_table_; diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index 7832fe970c87..9b3eb6a50ce1 100644 --- a/source/common/runtime/runtime_impl.h +++ b/source/common/runtime/runtime_impl.h @@ -268,7 +268,7 @@ class LoaderImpl : public Loader, Logger::Loggable { Upstream::ClusterManager* cm_{}; absl::Mutex snapshot_mutex_; - std::shared_ptr thread_safe_snapshot_ GUARDED_BY(snapshot_mutex_); + std::shared_ptr thread_safe_snapshot_ ABSL_GUARDED_BY(snapshot_mutex_); }; } // namespace Runtime diff --git a/source/common/upstream/thread_aware_lb_impl.h b/source/common/upstream/thread_aware_lb_impl.h index 2cf47f4e7552..3f68d1afbbaf 100644 --- a/source/common/upstream/thread_aware_lb_impl.h +++ b/source/common/upstream/thread_aware_lb_impl.h @@ -73,10 +73,10 @@ class ThreadAwareLoadBalancerBase : public LoadBalancerBase, public ThreadAwareL ClusterStats& stats_; Runtime::RandomGenerator& random_; absl::Mutex mutex_; - std::shared_ptr> per_priority_state_ GUARDED_BY(mutex_); + std::shared_ptr> per_priority_state_ ABSL_GUARDED_BY(mutex_); // This is split out of PerPriorityState so LoadBalancerBase::ChoosePriority can be reused. - std::shared_ptr healthy_per_priority_load_ GUARDED_BY(mutex_); - std::shared_ptr degraded_per_priority_load_ GUARDED_BY(mutex_); + std::shared_ptr healthy_per_priority_load_ ABSL_GUARDED_BY(mutex_); + std::shared_ptr degraded_per_priority_load_ ABSL_GUARDED_BY(mutex_); }; virtual HashingLoadBalancerSharedPtr diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 3f0f34d7545b..89c5071daa32 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -153,7 +153,7 @@ class HostDescriptionImpl : virtual public HostDescription { Network::Address::InstanceConstSharedPtr health_check_address_; std::atomic canary_; mutable absl::Mutex metadata_mutex_; - std::shared_ptr metadata_ GUARDED_BY(metadata_mutex_); + std::shared_ptr metadata_ ABSL_GUARDED_BY(metadata_mutex_); const envoy::api::v2::core::Locality locality_; Stats::StatNameManagedStorage locality_zone_stat_name_; Stats::IsolatedStoreImpl stats_store_; diff --git a/source/extensions/transport_sockets/tls/context_impl.h b/source/extensions/transport_sockets/tls/context_impl.h index ce5007fd5a34..a730187bcd54 100644 --- a/source/extensions/transport_sockets/tls/context_impl.h +++ b/source/extensions/transport_sockets/tls/context_impl.h @@ -194,7 +194,7 @@ class ClientContextImpl : public ContextImpl, public Envoy::Ssl::ClientContext { const bool allow_renegotiation_; const size_t max_session_keys_; absl::Mutex session_keys_mu_; - std::deque> session_keys_ GUARDED_BY(session_keys_mu_); + std::deque> session_keys_ ABSL_GUARDED_BY(session_keys_mu_); bool session_keys_single_use_{false}; }; diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index c7b183451b4f..4f7660717b80 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -152,7 +152,7 @@ class ClientSslSocketFactory : public Network::TransportSocketFactory, SslSocketFactoryStats stats_; Envoy::Ssl::ClientContextConfigPtr config_; mutable absl::Mutex ssl_ctx_mu_; - Envoy::Ssl::ClientContextSharedPtr ssl_ctx_ GUARDED_BY(ssl_ctx_mu_); + Envoy::Ssl::ClientContextSharedPtr ssl_ctx_ ABSL_GUARDED_BY(ssl_ctx_mu_); }; class ServerSslSocketFactory : public Network::TransportSocketFactory, @@ -177,7 +177,7 @@ class ServerSslSocketFactory : public Network::TransportSocketFactory, Envoy::Ssl::ServerContextConfigPtr config_; const std::vector server_names_; mutable absl::Mutex ssl_ctx_mu_; - Envoy::Ssl::ServerContextSharedPtr ssl_ctx_ GUARDED_BY(ssl_ctx_mu_); + Envoy::Ssl::ServerContextSharedPtr ssl_ctx_ ABSL_GUARDED_BY(ssl_ctx_mu_); }; } // namespace Tls diff --git a/source/server/guarddog_impl.h b/source/server/guarddog_impl.h index 5c0906ea55ad..6d50a2e23fed 100644 --- a/source/server/guarddog_impl.h +++ b/source/server/guarddog_impl.h @@ -115,13 +115,13 @@ class GuardDogImpl : public GuardDog { const std::chrono::milliseconds loop_interval_; Stats::Counter& watchdog_miss_counter_; Stats::Counter& watchdog_megamiss_counter_; - std::vector watched_dogs_ GUARDED_BY(wd_lock_); + std::vector watched_dogs_ ABSL_GUARDED_BY(wd_lock_); Thread::MutexBasicLockable wd_lock_; Thread::ThreadPtr thread_; Event::DispatcherPtr dispatcher_; Event::TimerPtr loop_timer_; Thread::MutexBasicLockable mutex_; - bool run_thread_ GUARDED_BY(mutex_); + bool run_thread_ ABSL_GUARDED_BY(mutex_); }; } // namespace Server diff --git a/test/common/common/lock_guard_test.cc b/test/common/common/lock_guard_test.cc index 163b535378b2..8cc8eb8b8355 100644 --- a/test/common/common/lock_guard_test.cc +++ b/test/common/common/lock_guard_test.cc @@ -9,7 +9,7 @@ namespace Thread { class ThreadTest : public testing::Test { protected: ThreadTest() = default; - int a_ GUARDED_BY(a_mutex_){0}; + int a_ ABSL_GUARDED_BY(a_mutex_){0}; MutexBasicLockable a_mutex_; int b_{0}; }; diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 444ae0c1195b..6257fedee966 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -293,8 +293,8 @@ class SharedConnectionWrapper : public Network::ConnectionCallbacks { private: Network::Connection& connection_; Thread::MutexBasicLockable lock_; - Common::CallbackManager<> disconnect_callback_manager_ GUARDED_BY(lock_); - bool disconnected_ GUARDED_BY(lock_){}; + Common::CallbackManager<> disconnect_callback_manager_ ABSL_GUARDED_BY(lock_); + bool disconnected_ ABSL_GUARDED_BY(lock_){}; const bool allow_unexpected_disconnects_; }; @@ -339,7 +339,7 @@ class QueuedConnectionWrapper : public LinkedObject { private: SharedConnectionWrapper shared_connection_; Thread::MutexBasicLockable lock_; - bool parented_ GUARDED_BY(lock_); + bool parented_ ABSL_GUARDED_BY(lock_); const bool allow_unexpected_disconnects_; }; @@ -399,7 +399,7 @@ class FakeConnectionBase : public Logger::Loggable { bool initialized_{}; Thread::CondVar connection_event_; Thread::MutexBasicLockable lock_; - bool half_closed_ GUARDED_BY(lock_){}; + bool half_closed_ ABSL_GUARDED_BY(lock_){}; Event::TestTimeSystem& time_system_; }; @@ -615,7 +615,7 @@ class FakeUpstream : Logger::Loggable, }; void threadRoutine(); - SharedConnectionWrapper& consumeConnection() EXCLUSIVE_LOCKS_REQUIRED(lock_); + SharedConnectionWrapper& consumeConnection() ABSL_EXCLUSIVE_LOCKS_REQUIRED(lock_); Network::SocketPtr socket_; ConditionalInitializer server_initialized_; @@ -628,11 +628,11 @@ class FakeUpstream : Logger::Loggable, Event::TestTimeSystem& time_system_; Event::DispatcherPtr dispatcher_; Network::ConnectionHandlerPtr handler_; - std::list new_connections_ GUARDED_BY(lock_); + std::list new_connections_ ABSL_GUARDED_BY(lock_); // When a QueuedConnectionWrapper is popped from new_connections_, ownership is transferred to // consumed_connections_. This allows later the Connection destruction (when the FakeUpstream is // deleted) on the same thread that allocated the connection. - std::list consumed_connections_ GUARDED_BY(lock_); + std::list consumed_connections_ ABSL_GUARDED_BY(lock_); bool allow_unexpected_disconnects_; bool read_disable_on_new_connection_; const bool enable_half_close_; diff --git a/test/integration/filters/pause_filter.cc b/test/integration/filters/pause_filter.cc index 8ed74d4198b8..c6463ef27b65 100644 --- a/test/integration/filters/pause_filter.cc +++ b/test/integration/filters/pause_filter.cc @@ -66,7 +66,8 @@ class TestPauseFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpF Http::FilterFactoryCb createFilter(const std::string&, Server::Configuration::FactoryContext&) override { return [&](Http::FilterChainFactoryCallbacks& callbacks) -> void { - // GUARDED_BY insists the lock be held when the guarded variables are passed by reference. + // ABSL_GUARDED_BY insists the lock be held when the guarded variables are passed by + // reference. absl::WriterMutexLock m(&encode_lock_); callbacks.addStreamFilter(std::make_shared<::Envoy::TestPauseFilter>( encode_lock_, number_of_encode_calls_, number_of_decode_calls_)); @@ -74,8 +75,8 @@ class TestPauseFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpF } absl::Mutex encode_lock_; - uint32_t number_of_encode_calls_ GUARDED_BY(encode_lock_) = 0; - uint32_t number_of_decode_calls_ GUARDED_BY(encode_lock_) = 0; + uint32_t number_of_encode_calls_ ABSL_GUARDED_BY(encode_lock_) = 0; + uint32_t number_of_decode_calls_ ABSL_GUARDED_BY(encode_lock_) = 0; }; // perform static registration diff --git a/test/integration/filters/random_pause_filter.cc b/test/integration/filters/random_pause_filter.cc index b709da758ce9..24609cf70457 100644 --- a/test/integration/filters/random_pause_filter.cc +++ b/test/integration/filters/random_pause_filter.cc @@ -62,7 +62,7 @@ class RandomPauseFilterConfig : public Extensions::HttpFilters::Common::EmptyHtt } absl::Mutex rand_lock_; - std::unique_ptr rng_ GUARDED_BY(rand_lock_); + std::unique_ptr rng_ ABSL_GUARDED_BY(rand_lock_); }; // perform static registration diff --git a/test/mocks/common.h b/test/mocks/common.h index 1cbffa2eeded..c8cda0e44794 100644 --- a/test/mocks/common.h +++ b/test/mocks/common.h @@ -56,7 +56,7 @@ class MockTimeSystem : public Event::TestTimeSystem { void sleep(const Duration& duration) override { real_time_.sleep(duration); } Thread::CondVar::WaitStatus waitFor(Thread::MutexBasicLockable& mutex, Thread::CondVar& condvar, - const Duration& duration) noexcept EXCLUSIVE_LOCKS_REQUIRED(mutex) override { + const Duration& duration) noexcept ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex) override { return real_time_.waitFor(mutex, condvar, duration); // NO_CHECK_FORMAT(real_time) } MOCK_METHOD0(systemTime, SystemTime()); diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc index e4f5d35f8e2c..66f5b117121f 100644 --- a/test/server/guarddog_impl_test.cc +++ b/test/server/guarddog_impl_test.cc @@ -36,7 +36,7 @@ class DebugTestInterlock : public GuardDogImpl::TestInterlockHook { } void waitFromTest(Thread::MutexBasicLockable& mutex, MonotonicTime time) override - EXCLUSIVE_LOCKS_REQUIRED(mutex) { + ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex) { while (impl_reached_ < time) { impl_.wait(mutex); }