Skip to content

Commit

Permalink
Replace deprecated thread annotations macros. (envoyproxy#8237)
Browse files Browse the repository at this point in the history
Abseil thread annotation macros are now prefixed by ABSL_.

There is no semantic change; this is just a rename.

Signed-off-by: Yan Avlasov <yavlasov@google.com>
  • Loading branch information
yanavlasov authored and danzh1989 committed Sep 24, 2019
1 parent 2a0ab6d commit b50e241
Show file tree
Hide file tree
Showing 19 changed files with 57 additions and 55 deletions.
8 changes: 4 additions & 4 deletions include/envoy/thread/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions source/common/access_log/access_log_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ class AccessLogFileImpl : public AccessLogFile {
std::atomic<bool> flush_thread_exit_{};
std::atomic<bool> 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
Expand Down
23 changes: 12 additions & 11 deletions source/common/common/lock_guard.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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
Expand Down Expand Up @@ -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_;
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class DelegatingLogSink : public spdlog::sinks::sink {

SinkDelegate* sink_{nullptr};
std::unique_ptr<StderrSinkDelegate> stderr_sink_; // Builtin sink to use as a last resort.
std::unique_ptr<spdlog::formatter> formatter_ GUARDED_BY(format_mutex_);
std::unique_ptr<spdlog::formatter> formatter_ ABSL_GUARDED_BY(format_mutex_);
absl::Mutex format_mutex_; // direct absl reference to break build cycle.
};

Expand Down
14 changes: 7 additions & 7 deletions source/common/common/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,17 +52,17 @@ 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 <class Rep, class Period>

/**
* @return WaitStatus whether the condition timed out or not.
*/
WaitStatus
waitFor(MutexBasicLockable& mutex,
std::chrono::duration<Rep, Period> duration) noexcept EXCLUSIVE_LOCKS_REQUIRED(mutex) {
WaitStatus waitFor(
MutexBasicLockable& mutex,
std::chrono::duration<Rep, Period> duration) noexcept ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex) {
return condvar_.WaitWithTimeout(&mutex.mutex_, absl::FromChrono(duration))
? WaitStatus::Timeout
: WaitStatus::NoTimeout;
Expand Down
2 changes: 1 addition & 1 deletion source/common/event/dispatcher_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class DispatcherImpl : Logger::Loggable<Logger::Id::main>,
std::vector<DeferredDeletablePtr> to_delete_2_;
std::vector<DeferredDeletablePtr>* current_to_delete_;
Thread::MutexBasicLockable post_lock_;
std::list<std::function<void()>> post_callbacks_ GUARDED_BY(post_lock_);
std::list<std::function<void()>> post_callbacks_ ABSL_GUARDED_BY(post_lock_);
const ScopeTrackedObject* current_object_{};
bool deferred_deleting_{};
};
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/codes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand Down
2 changes: 1 addition & 1 deletion source/common/runtime/runtime_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class LoaderImpl : public Loader, Logger::Loggable<Logger::Id::runtime> {
Upstream::ClusterManager* cm_{};

absl::Mutex snapshot_mutex_;
std::shared_ptr<const Snapshot> thread_safe_snapshot_ GUARDED_BY(snapshot_mutex_);
std::shared_ptr<const Snapshot> thread_safe_snapshot_ ABSL_GUARDED_BY(snapshot_mutex_);
};

} // namespace Runtime
Expand Down
6 changes: 3 additions & 3 deletions source/common/upstream/thread_aware_lb_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ class ThreadAwareLoadBalancerBase : public LoadBalancerBase, public ThreadAwareL
ClusterStats& stats_;
Runtime::RandomGenerator& random_;
absl::Mutex mutex_;
std::shared_ptr<std::vector<PerPriorityStatePtr>> per_priority_state_ GUARDED_BY(mutex_);
std::shared_ptr<std::vector<PerPriorityStatePtr>> per_priority_state_ ABSL_GUARDED_BY(mutex_);
// This is split out of PerPriorityState so LoadBalancerBase::ChoosePriority can be reused.
std::shared_ptr<HealthyLoad> healthy_per_priority_load_ GUARDED_BY(mutex_);
std::shared_ptr<DegradedLoad> degraded_per_priority_load_ GUARDED_BY(mutex_);
std::shared_ptr<HealthyLoad> healthy_per_priority_load_ ABSL_GUARDED_BY(mutex_);
std::shared_ptr<DegradedLoad> degraded_per_priority_load_ ABSL_GUARDED_BY(mutex_);
};

virtual HashingLoadBalancerSharedPtr
Expand Down
2 changes: 1 addition & 1 deletion source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class HostDescriptionImpl : virtual public HostDescription {
Network::Address::InstanceConstSharedPtr health_check_address_;
std::atomic<bool> canary_;
mutable absl::Mutex metadata_mutex_;
std::shared_ptr<envoy::api::v2::core::Metadata> metadata_ GUARDED_BY(metadata_mutex_);
std::shared_ptr<envoy::api::v2::core::Metadata> metadata_ ABSL_GUARDED_BY(metadata_mutex_);
const envoy::api::v2::core::Locality locality_;
Stats::StatNameManagedStorage locality_zone_stat_name_;
Stats::IsolatedStoreImpl stats_store_;
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/transport_sockets/tls/context_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<bssl::UniquePtr<SSL_SESSION>> session_keys_ GUARDED_BY(session_keys_mu_);
std::deque<bssl::UniquePtr<SSL_SESSION>> session_keys_ ABSL_GUARDED_BY(session_keys_mu_);
bool session_keys_single_use_{false};
};

Expand Down
4 changes: 2 additions & 2 deletions source/extensions/transport_sockets/tls/ssl_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -177,7 +177,7 @@ class ServerSslSocketFactory : public Network::TransportSocketFactory,
Envoy::Ssl::ServerContextConfigPtr config_;
const std::vector<std::string> 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
Expand Down
4 changes: 2 additions & 2 deletions source/server/guarddog_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<WatchedDog> watched_dogs_ GUARDED_BY(wd_lock_);
std::vector<WatchedDog> 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
Expand Down
2 changes: 1 addition & 1 deletion test/common/common/lock_guard_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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};
};
Expand Down
14 changes: 7 additions & 7 deletions test/integration/fake_upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
};

Expand Down Expand Up @@ -339,7 +339,7 @@ class QueuedConnectionWrapper : public LinkedObject<QueuedConnectionWrapper> {
private:
SharedConnectionWrapper shared_connection_;
Thread::MutexBasicLockable lock_;
bool parented_ GUARDED_BY(lock_);
bool parented_ ABSL_GUARDED_BY(lock_);
const bool allow_unexpected_disconnects_;
};

Expand Down Expand Up @@ -399,7 +399,7 @@ class FakeConnectionBase : public Logger::Loggable<Logger::Id::testing> {
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_;
};

Expand Down Expand Up @@ -615,7 +615,7 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
};

void threadRoutine();
SharedConnectionWrapper& consumeConnection() EXCLUSIVE_LOCKS_REQUIRED(lock_);
SharedConnectionWrapper& consumeConnection() ABSL_EXCLUSIVE_LOCKS_REQUIRED(lock_);

Network::SocketPtr socket_;
ConditionalInitializer server_initialized_;
Expand All @@ -628,11 +628,11 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
Event::TestTimeSystem& time_system_;
Event::DispatcherPtr dispatcher_;
Network::ConnectionHandlerPtr handler_;
std::list<QueuedConnectionWrapperPtr> new_connections_ GUARDED_BY(lock_);
std::list<QueuedConnectionWrapperPtr> 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<QueuedConnectionWrapperPtr> consumed_connections_ GUARDED_BY(lock_);
std::list<QueuedConnectionWrapperPtr> consumed_connections_ ABSL_GUARDED_BY(lock_);
bool allow_unexpected_disconnects_;
bool read_disable_on_new_connection_;
const bool enable_half_close_;
Expand Down
7 changes: 4 additions & 3 deletions test/integration/filters/pause_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,17 @@ 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_));
};
}

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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/filters/random_pause_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class RandomPauseFilterConfig : public Extensions::HttpFilters::Common::EmptyHtt
}

absl::Mutex rand_lock_;
std::unique_ptr<TestRandomGenerator> rng_ GUARDED_BY(rand_lock_);
std::unique_ptr<TestRandomGenerator> rng_ ABSL_GUARDED_BY(rand_lock_);
};

// perform static registration
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion test/server/guarddog_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit b50e241

Please sign in to comment.