Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated thread annotations macros. #8237

Merged
merged 1 commit into from
Sep 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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