From 8545b413cac990e8545e9898186fc353f03ef3c2 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 17 Oct 2017 11:42:57 -0400 Subject: [PATCH 01/10] WIP Signed-off-by: Alyssa Wilk --- source/common/api/BUILD | 1 + source/common/api/api_impl.cc | 4 ++-- source/common/api/api_impl.h | 2 +- source/common/api/os_sys_calls_impl.h | 4 ++++ source/common/common/singleton.h | 8 ++++++-- source/server/BUILD | 1 + source/server/hot_restart_impl.cc | 13 +++++++++---- 7 files changed, 24 insertions(+), 9 deletions(-) diff --git a/source/common/api/BUILD b/source/common/api/BUILD index 3e54452e2511..07db89a5e033 100644 --- a/source/common/api/BUILD +++ b/source/common/api/BUILD @@ -26,5 +26,6 @@ envoy_cc_library( hdrs = ["os_sys_calls_impl.h"], deps = [ "//include/envoy/api:os_sys_calls_interface", + "//source/common/common:singleton", ], ) diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 190eb723e5fe..d5018daaf3fa 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -15,11 +15,11 @@ Event::DispatcherPtr Impl::allocateDispatcher() { } Impl::Impl(std::chrono::milliseconds file_flush_interval_msec) - : os_sys_calls_(new OsSysCallsImpl()), file_flush_interval_msec_(file_flush_interval_msec) {} + : os_sys_calls_(OsSysCallsSingleton::get()), file_flush_interval_msec_(file_flush_interval_msec) {} Filesystem::FileSharedPtr Impl::createFile(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, Stats::Store& stats_store) { - return std::make_shared(path, dispatcher, lock, *os_sys_calls_, stats_store, + return std::make_shared(path, dispatcher, lock, os_sys_calls_, stats_store, file_flush_interval_msec_); } diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index f0532bac9d81..eab2ef2ad077 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -26,7 +26,7 @@ class Impl : public Api::Api { std::string fileReadToEnd(const std::string& path) override; private: - OsSysCallsPtr os_sys_calls_; + OsSysCalls& os_sys_calls_; std::chrono::milliseconds file_flush_interval_msec_; }; diff --git a/source/common/api/os_sys_calls_impl.h b/source/common/api/os_sys_calls_impl.h index e14be7df3bf2..21c9b6f0ca4d 100644 --- a/source/common/api/os_sys_calls_impl.h +++ b/source/common/api/os_sys_calls_impl.h @@ -2,6 +2,8 @@ #include "envoy/api/os_sys_calls.h" +#include "common/common/singleton.h" + namespace Envoy { namespace Api { @@ -18,5 +20,7 @@ class OsSysCallsImpl : public OsSysCalls { void* mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) override; }; +typedef ConstSingleton OsSysCallsSingleton; + } // namespace Api } // namespace Envoy diff --git a/source/common/common/singleton.h b/source/common/common/singleton.h index ee50c0a75195..b34ea56e32ae 100644 --- a/source/common/common/singleton.h +++ b/source/common/common/singleton.h @@ -2,6 +2,7 @@ namespace Envoy { +// FIXME(alyssar) ThreadSafeSingleton? /** * Immutable singleton pattern. See singleton/manager.h for mutable/destroyable singletons. */ @@ -9,9 +10,12 @@ template class ConstSingleton { public: /** * Obtain an instance of the singleton for class T. - * @return const T& a reference to the singleton for class T. + * + * All functions in T must be thread-safe. + * + * @return T& a reference to the singleton for class T. */ - static const T& get() { + static T& get() { static T* instance = new T(); return *instance; } diff --git a/source/server/BUILD b/source/server/BUILD index e33387e6a8f3..63e19a35e694 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -113,6 +113,7 @@ envoy_cc_library( "//include/envoy/server:hot_restart_interface", "//include/envoy/server:instance_interface", "//include/envoy/server:options_interface", + "//source/common/api:os_sys_calls_lib", "//source/common/common:assert_lib", "//source/common/common:utility_lib", "//source/common/network:utility_lib", diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc index dc1d45f9a15c..29a879831449 100644 --- a/source/server/hot_restart_impl.cc +++ b/source/server/hot_restart_impl.cc @@ -13,6 +13,7 @@ #include "envoy/server/instance.h" #include "envoy/server/options.h" +#include "common/api/os_sys_calls_impl.h" #include "common/common/utility.h" #include "common/network/utility.h" @@ -25,7 +26,9 @@ namespace Server { // from working. Operations code can then cope with this and do a full restart. const uint64_t SharedMemory::VERSION = 9; -SharedMemory& SharedMemory::initialize(Options& options, Api::OsSysCalls& os_sys_calls) { +SharedMemory& SharedMemory::initialize(Options& options, Api::OsSysCalls&) { + Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); + const uint64_t entry_size = Stats::RawStatData::size(); const uint64_t total_size = sizeof(SharedMemory) + (entry_size * options.maxStats()); @@ -105,10 +108,11 @@ std::string SharedMemory::version() { return version(num_stats_, Stats::RawStatData::maxNameLength()); } -HotRestartImpl::HotRestartImpl(Options& options, Api::OsSysCalls& os_sys_calls) - : options_(options), shmem_(SharedMemory::initialize(options, os_sys_calls)), +HotRestartImpl::HotRestartImpl(Options& options, Api::OsSysCalls&) + : options_(options), shmem_(SharedMemory::initialize(options, Api::OsSysCallsSingleton::get())), log_lock_(shmem_.log_lock_), access_log_lock_(shmem_.access_log_lock_), stat_lock_(shmem_.stat_lock_), init_lock_(shmem_.init_lock_) { + Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); my_domain_socket_ = bindDomainSocket(options.restartEpoch(), os_sys_calls); child_address_ = createDomainSocketAddress((options.restartEpoch() + 1)); @@ -157,7 +161,8 @@ void HotRestartImpl::free(Stats::RawStatData& data) { memset(&data, 0, Stats::RawStatData::size()); } -int HotRestartImpl::bindDomainSocket(uint64_t id, Api::OsSysCalls& os_sys_calls) { +int HotRestartImpl::bindDomainSocket(uint64_t id, Api::OsSysCalls&) { + Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); // This actually creates the socket and binds it. We use the socket in datagram mode so we can // easily read single messages. int fd = socket(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0); From f5b6174768310023dd484e60b9174db5eac0160c Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 17 Oct 2017 13:09:37 -0400 Subject: [PATCH 02/10] WIP Signed-off-by: Alyssa Wilk --- bazel/repositories.bzl | 15 +++++++++++++++ bazel/repository_locations.bzl | 1 + 2 files changed, 16 insertions(+) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 5dabee7f4ddd..b2ee8a5c779c 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -143,6 +143,20 @@ def envoy_api_deps(skip_targets): actual = "@googleapis//:http_api_protos_genproto", ) + +def abseil_deps(skip_targets): + if 'abseil' not in skip_targets: + native.git_repository( + name = "abseil", + remote = REPO_LOCATIONS["abseil_cpp"], + commit = "6de53819a7173bd446156237a37f53464b7732cc", + ) + native.bind( + name = "abseil_base", + actual = "@abseil//absl/base:base", + ) + + def envoy_dependencies(path = "@envoy_deps//", skip_protobuf_bzl = False, skip_targets = [], repository = ""): native.bind( @@ -209,6 +223,7 @@ def envoy_dependencies(path = "@envoy_deps//", skip_protobuf_bzl = False, skip_t python_deps(skip_targets) cc_deps(skip_targets) envoy_api_deps(skip_targets) + abseil_deps(skip_targets) def com_github_fmtlib_fmt(repository = ""): native.new_http_archive( diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 795fbae20d35..7efae2f8befb 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -3,4 +3,5 @@ REPO_LOCATIONS = { "grpc_transcoding": "https://github.com/grpc-ecosystem/grpc-httpjson-transcoding.git", "envoy_api": "https://github.com/envoyproxy/data-plane-api.git", "markupsafe": "https://github.com/pallets/markupsafe.git", + "abseil_cpp": "https://github.com/abseil/abseil-cpp", } From 7abd66c021e8af9b10246f875d79e1dc1f19a990 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 18 Oct 2017 09:21:01 -0400 Subject: [PATCH 03/10] WIP Signed-off-by: Alyssa Wilk --- source/common/api/api_impl.cc | 5 +-- source/common/api/api_impl.h | 2 - source/common/api/os_sys_calls_impl.h | 2 +- source/common/common/BUILD | 1 + source/common/common/singleton.h | 39 +++++++++++++++---- source/common/filesystem/BUILD | 1 + source/common/filesystem/filesystem_impl.cc | 7 ++-- source/common/filesystem/filesystem_impl.h | 3 +- source/exe/main_common.cc | 4 +- source/server/hot_restart_impl.cc | 12 +++--- source/server/hot_restart_impl.h | 7 ++-- test/common/filesystem/BUILD | 1 + .../common/filesystem/filesystem_impl_test.cc | 10 +++-- test/server/BUILD | 2 + test/server/hot_restart_impl_test.cc | 8 +++- test/test_common/BUILD | 8 ++++ test/test_common/singleton_injector.h | 19 +++++++++ 17 files changed, 92 insertions(+), 39 deletions(-) create mode 100644 test/test_common/singleton_injector.h diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index d5018daaf3fa..86f418939e10 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -3,7 +3,6 @@ #include #include -#include "common/api/os_sys_calls_impl.h" #include "common/event/dispatcher_impl.h" #include "common/filesystem/filesystem_impl.h" @@ -15,11 +14,11 @@ Event::DispatcherPtr Impl::allocateDispatcher() { } Impl::Impl(std::chrono::milliseconds file_flush_interval_msec) - : os_sys_calls_(OsSysCallsSingleton::get()), file_flush_interval_msec_(file_flush_interval_msec) {} + : file_flush_interval_msec_(file_flush_interval_msec) {} Filesystem::FileSharedPtr Impl::createFile(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, Stats::Store& stats_store) { - return std::make_shared(path, dispatcher, lock, os_sys_calls_, stats_store, + return std::make_shared(path, dispatcher, lock, stats_store, file_flush_interval_msec_); } diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index eab2ef2ad077..b7f0f6fb609a 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -4,7 +4,6 @@ #include #include "envoy/api/api.h" -#include "envoy/api/os_sys_calls.h" #include "envoy/filesystem/filesystem.h" namespace Envoy { @@ -26,7 +25,6 @@ class Impl : public Api::Api { std::string fileReadToEnd(const std::string& path) override; private: - OsSysCalls& os_sys_calls_; std::chrono::milliseconds file_flush_interval_msec_; }; diff --git a/source/common/api/os_sys_calls_impl.h b/source/common/api/os_sys_calls_impl.h index 21c9b6f0ca4d..24d9774a1a32 100644 --- a/source/common/api/os_sys_calls_impl.h +++ b/source/common/api/os_sys_calls_impl.h @@ -20,7 +20,7 @@ class OsSysCallsImpl : public OsSysCalls { void* mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) override; }; -typedef ConstSingleton OsSysCallsSingleton; +typedef ThreadSafeSingleton OsSysCallsSingleton; } // namespace Api } // namespace Envoy diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 574ad39995b9..f3f3ecea1236 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -95,6 +95,7 @@ envoy_cc_library( envoy_cc_library( name = "singleton", hdrs = ["singleton.h"], + external_deps = ["abseil_base"], ) envoy_cc_library( diff --git a/source/common/common/singleton.h b/source/common/common/singleton.h index b34ea56e32ae..c2b1d4c35b7d 100644 --- a/source/common/common/singleton.h +++ b/source/common/common/singleton.h @@ -1,8 +1,9 @@ #pragma once +#include "absl/base/call_once.h" + namespace Envoy { -// FIXME(alyssar) ThreadSafeSingleton? /** * Immutable singleton pattern. See singleton/manager.h for mutable/destroyable singletons. */ @@ -10,15 +11,37 @@ template class ConstSingleton { public: /** * Obtain an instance of the singleton for class T. - * - * All functions in T must be thread-safe. - * - * @return T& a reference to the singleton for class T. + * @return const T& a reference to the singleton for class T. */ + static const T& get() { + absl::call_once(create_once_, &ConstSingleton::Create); + return *instance_; + } + +protected: + template friend class TestThreadsafeSingletonInjector; + + static void Create() { + instance_ = new T(); + } + + static absl::once_flag create_once_; + static T* instance_; +}; + +/* Mutable singleton. All functions in the singleton class *must* be threadsafe.*/ +template class ThreadSafeSingleton : public ConstSingleton { + public: static T& get() { - static T* instance = new T(); - return *instance; + absl::call_once(ConstSingleton::create_once_, &ConstSingleton::Create); + return *ConstSingleton::instance_; } }; -} // namespace Envoy +template +absl::once_flag ConstSingleton::create_once_; + +template +T* ConstSingleton::instance_ = nullptr; + +} // namespace Envoy diff --git a/source/common/filesystem/BUILD b/source/common/filesystem/BUILD index 9c386440935b..d08e3a73c53d 100644 --- a/source/common/filesystem/BUILD +++ b/source/common/filesystem/BUILD @@ -16,6 +16,7 @@ envoy_cc_library( "//include/envoy/api:os_sys_calls_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/filesystem:filesystem_interface", + "//source/common/api:os_sys_calls_lib", "//source/common/buffer:buffer_lib", "//source/common/common:thread_lib", ], diff --git a/source/common/filesystem/filesystem_impl.cc b/source/common/filesystem/filesystem_impl.cc index 8ea9b3e6c8a2..3c77556da258 100644 --- a/source/common/filesystem/filesystem_impl.cc +++ b/source/common/filesystem/filesystem_impl.cc @@ -15,6 +15,7 @@ #include "envoy/event/timer.h" #include "envoy/stats/stats.h" +#include "common/api/os_sys_calls_impl.h" #include "common/common/assert.h" #include "common/common/thread.h" @@ -53,14 +54,14 @@ std::string fileReadToEnd(const std::string& path) { } FileImpl::FileImpl(const std::string& path, Event::Dispatcher& dispatcher, - Thread::BasicLockable& lock, Api::OsSysCalls& os_sys_calls, - Stats::Store& stats_store, std::chrono::milliseconds flush_interval_msec) + Thread::BasicLockable& lock, Stats::Store& stats_store, + std::chrono::milliseconds flush_interval_msec) : path_(path), file_lock_(lock), flush_timer_(dispatcher.createTimer([this]() -> void { stats_.flushed_by_timer_.inc(); flush_event_.notify_one(); flush_timer_->enableTimer(flush_interval_msec_); })), - os_sys_calls_(os_sys_calls), flush_interval_msec_(flush_interval_msec), + os_sys_calls_(Api::OsSysCallsSingleton::get()), flush_interval_msec_(flush_interval_msec), stats_{FILESYSTEM_STATS(POOL_COUNTER_PREFIX(stats_store, "filesystem."), POOL_GAUGE_PREFIX(stats_store, "filesystem."))} { open(); diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/filesystem_impl.h index 84dce1a58111..4d154a295011 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/filesystem_impl.h @@ -57,8 +57,7 @@ std::string fileReadToEnd(const std::string& path); class FileImpl : public File { public: FileImpl(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, - Api::OsSysCalls& osSysCalls, Stats::Store& stats_store, - std::chrono::milliseconds flush_interval_msec); + Stats::Store& stats_store, std::chrono::milliseconds flush_interval_msec); ~FileImpl(); // Filesystem::File diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 18ebbfee8619..50c183c66c01 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -15,7 +15,6 @@ #include "server/test_hooks.h" #ifdef ENVOY_HOT_RESTART -#include "common/api/os_sys_calls_impl.h" #include "server/hot_restart_impl.h" #endif @@ -43,10 +42,9 @@ int main_common(OptionsImpl& options) { Stats::RawStatData::configure(options); #ifdef ENVOY_HOT_RESTART - Api::OsSysCallsImpl os_sys_calls_impl; std::unique_ptr restarter; try { - restarter.reset(new Server::HotRestartImpl(options, os_sys_calls_impl)); + restarter.reset(new Server::HotRestartImpl(options)); } catch (Envoy::EnvoyException& e) { std::cerr << "unable to initialize hot restart: " << e.what() << std::endl; return 1; diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc index 29a879831449..8e980935c509 100644 --- a/source/server/hot_restart_impl.cc +++ b/source/server/hot_restart_impl.cc @@ -26,7 +26,7 @@ namespace Server { // from working. Operations code can then cope with this and do a full restart. const uint64_t SharedMemory::VERSION = 9; -SharedMemory& SharedMemory::initialize(Options& options, Api::OsSysCalls&) { +SharedMemory& SharedMemory::initialize(Options& options) { Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); const uint64_t entry_size = Stats::RawStatData::size(); @@ -108,13 +108,11 @@ std::string SharedMemory::version() { return version(num_stats_, Stats::RawStatData::maxNameLength()); } -HotRestartImpl::HotRestartImpl(Options& options, Api::OsSysCalls&) - : options_(options), shmem_(SharedMemory::initialize(options, Api::OsSysCallsSingleton::get())), +HotRestartImpl::HotRestartImpl(Options& options) + : options_(options), shmem_(SharedMemory::initialize(options)), log_lock_(shmem_.log_lock_), access_log_lock_(shmem_.access_log_lock_), stat_lock_(shmem_.stat_lock_), init_lock_(shmem_.init_lock_) { - Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); - - my_domain_socket_ = bindDomainSocket(options.restartEpoch(), os_sys_calls); + my_domain_socket_ = bindDomainSocket(options.restartEpoch()); child_address_ = createDomainSocketAddress((options.restartEpoch() + 1)); if (options.restartEpoch() != 0) { parent_address_ = createDomainSocketAddress((options.restartEpoch() + -1)); @@ -161,7 +159,7 @@ void HotRestartImpl::free(Stats::RawStatData& data) { memset(&data, 0, Stats::RawStatData::size()); } -int HotRestartImpl::bindDomainSocket(uint64_t id, Api::OsSysCalls&) { +int HotRestartImpl::bindDomainSocket(uint64_t id) { Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); // This actually creates the socket and binds it. We use the socket in datagram mode so we can // easily read single messages. diff --git a/source/server/hot_restart_impl.h b/source/server/hot_restart_impl.h index 0c6205ea6659..2270c77b757c 100644 --- a/source/server/hot_restart_impl.h +++ b/source/server/hot_restart_impl.h @@ -8,7 +8,6 @@ #include #include -#include "envoy/api/os_sys_calls.h" #include "envoy/server/hot_restart.h" #include "envoy/server/options.h" @@ -42,7 +41,7 @@ class SharedMemory { * Initialize the shared memory segment, depending on whether we should be the first running * envoy, or a host restarted envoy process. */ - static SharedMemory& initialize(Options& options, Api::OsSysCalls& os_sys_calls); + static SharedMemory& initialize(Options& options); /** * Initialize a pthread mutex for process shared locking. @@ -116,7 +115,7 @@ class HotRestartImpl : public HotRestart, public Stats::RawStatDataAllocator, Logger::Loggable { public: - HotRestartImpl(Options& options, Api::OsSysCalls& os_sys_calls); + HotRestartImpl(Options& options); Thread::BasicLockable& logLock() { return log_lock_; } Thread::BasicLockable& accessLogLock() { return access_log_lock_; } @@ -189,7 +188,7 @@ class HotRestartImpl : public HotRestart, return reinterpret_cast(base_message); } - int bindDomainSocket(uint64_t id, Api::OsSysCalls& os_sys_calls); + int bindDomainSocket(uint64_t id); sockaddr_un createDomainSocketAddress(uint64_t id); void onGetListenSocket(RpcGetListenSocketRequest& rpc); void onSocketEvent(); diff --git a/test/common/filesystem/BUILD b/test/common/filesystem/BUILD index d7f9a8c471f1..21749ec136c2 100644 --- a/test/common/filesystem/BUILD +++ b/test/common/filesystem/BUILD @@ -22,6 +22,7 @@ envoy_cc_test( "//test/mocks/event:event_mocks", "//test/mocks/filesystem:filesystem_mocks", "//test/test_common:environment_lib", + "//test/test_common:singleton_injector_lib", ], ) diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index 36606c5277eb..6e35a6db6d40 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -11,6 +11,7 @@ #include "test/mocks/event/mocks.h" #include "test/mocks/filesystem/mocks.h" #include "test/test_common/environment.h" +#include "test/test_common/singleton_injector.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -30,9 +31,8 @@ TEST(FileSystemImpl, BadFile) { Event::MockDispatcher dispatcher; Thread::MutexBasicLockable lock; Stats::IsolatedStoreImpl store; - Api::OsSysCallsImpl os_sys_calls; EXPECT_CALL(dispatcher, createTimer_(_)); - EXPECT_THROW(Filesystem::FileImpl("", dispatcher, lock, os_sys_calls, store, + EXPECT_THROW(Filesystem::FileImpl("", dispatcher, lock, store, std::chrono::milliseconds(10000)), EnvoyException); } @@ -85,9 +85,10 @@ TEST(FileSystemImpl, flushToLogFilePeriodically) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; + TestThreadsafeSingletonInjector os_calls(&os_sys_calls_); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, os_sys_calls, stats_store, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); @@ -138,9 +139,10 @@ TEST(FileSystemImpl, flushToLogFileOnDemand) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; + TestThreadsafeSingletonInjector os_calls(&os_sys_calls_); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, os_sys_calls, stats_store, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); diff --git a/test/server/BUILD b/test/server/BUILD index d338371f65dd..b05db9a3a80c 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -59,8 +59,10 @@ envoy_cc_test( name = "hot_restart_impl_test", srcs = envoy_select_hot_restart(["hot_restart_impl_test.cc"]), deps = [ + "//source/common/api:os_sys_calls_lib", "//source/server:hot_restart_lib", "//test/mocks/server:server_mocks", + "//test/test_common:singleton_injector_lib", ], ) diff --git a/test/server/hot_restart_impl_test.cc b/test/server/hot_restart_impl_test.cc index 2ef43ac122f4..233646a42393 100644 --- a/test/server/hot_restart_impl_test.cc +++ b/test/server/hot_restart_impl_test.cc @@ -1,7 +1,10 @@ #include "server/hot_restart_impl.h" +#include "common/api/os_sys_calls_impl.h" + #include "test/mocks/api/mocks.h" #include "test/mocks/server/mocks.h" +#include "test/test_common/singleton_injector.h" #include "gtest/gtest.h" @@ -31,7 +34,7 @@ class HotRestartImplTest : public testing::Test { Stats::RawStatData::configureForTestsOnly(options_); // Test we match the correct stat with empty-slots before, after, or both. - hot_restart_.reset(new HotRestartImpl(options_, os_sys_calls_)); + hot_restart_.reset(new HotRestartImpl(options_)); hot_restart_->drainParentListeners(); } @@ -43,6 +46,7 @@ class HotRestartImplTest : public testing::Test { } Api::MockOsSysCalls os_sys_calls_; + TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; NiceMock options_; std::vector buffer_; std::unique_ptr hot_restart_; @@ -71,7 +75,7 @@ TEST_F(HotRestartImplTest, crossAlloc) { EXPECT_CALL(os_sys_calls_, shmOpen(_, _, _)); EXPECT_CALL(os_sys_calls_, mmap(_, _, _, _, _, _)).WillOnce(Return(buffer_.data())); EXPECT_CALL(os_sys_calls_, bind(_, _, _)); - HotRestartImpl hot_restart2(options_, os_sys_calls_); + HotRestartImpl hot_restart2(options_); Stats::RawStatData* stat1_prime = hot_restart2.alloc("stat1"); Stats::RawStatData* stat3_prime = hot_restart2.alloc("stat3"); Stats::RawStatData* stat5_prime = hot_restart2.alloc("stat5"); diff --git a/test/test_common/BUILD b/test/test_common/BUILD index 93c1edee217f..1c05ebe6bba0 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -74,3 +74,11 @@ envoy_cc_test_library( "//source/common/protobuf:utility_lib", ], ) + +envoy_cc_library( + name = "singleton_injector_lib", + hdrs = ["singleton_injector.h"], + deps = [ + "//source/common/common:singleton", + ], +) diff --git a/test/test_common/singleton_injector.h b/test/test_common/singleton_injector.h new file mode 100644 index 000000000000..b2c45924c4be --- /dev/null +++ b/test/test_common/singleton_injector.h @@ -0,0 +1,19 @@ +#include "common/common/singleton.h" + +namespace Envoy { + +// Note this class is not thread-safe, and should be called exceedingly carefully. +template class TestThreadsafeSingletonInjector { +public: + TestThreadsafeSingletonInjector(T* instance) { + latched_instance_ = ThreadSafeSingleton::get(); + ThreadSafeSingleton::instance_ = instance; + } + ~TestThreadsafeSingletonInjector() { + ThreadSafeSingleton::instance_ = latched_instance_; + } + private: + static T* latched_instance_; +}; + +} // namespace Envoy From 1f5785098ff22c79251998760c8beec849bcb54a Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 19 Oct 2017 15:20:24 -0400 Subject: [PATCH 04/10] status Signed-off-by: Alyssa Wilk --- test/common/filesystem/filesystem_impl_test.cc | 13 ++++++++----- test/mocks/api/BUILD | 1 + test/mocks/api/mocks.h | 4 +++- test/test_common/singleton_injector.h | 4 ++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index 6e35a6db6d40..97fe507f040c 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -85,7 +85,7 @@ TEST(FileSystemImpl, flushToLogFilePeriodically) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; - TestThreadsafeSingletonInjector os_calls(&os_sys_calls_); + TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); Filesystem::FileImpl file("", dispatcher, mutex, stats_store, @@ -139,7 +139,7 @@ TEST(FileSystemImpl, flushToLogFileOnDemand) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; - TestThreadsafeSingletonInjector os_calls(&os_sys_calls_); + TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); Filesystem::FileImpl file("", dispatcher, mutex, stats_store, @@ -213,10 +213,11 @@ TEST(FileSystemImpl, reopenFile) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; + TestThreadsafeSingletonInjector os_calls(&os_sys_calls); Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, os_sys_calls, stats_store, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) @@ -273,6 +274,7 @@ TEST(FilesystemImpl, reopenThrows) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; + TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, write_(_, _, _)) .WillRepeatedly(Invoke([](int fd, const void* buffer, size_t num_bytes) -> ssize_t { @@ -285,7 +287,7 @@ TEST(FilesystemImpl, reopenThrows) { Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, os_sys_calls, stats_store, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, close(5)).InSequence(sq); EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(-1)); @@ -320,8 +322,9 @@ TEST(FilesystemImpl, bigDataChunkShouldBeFlushedWithoutTimer) { Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; + TestThreadsafeSingletonInjector os_calls(&os_sys_calls); - Filesystem::FileImpl file("", dispatcher, mutex, os_sys_calls, stats_store, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) diff --git a/test/mocks/api/BUILD b/test/mocks/api/BUILD index def769daa439..5325b4865934 100644 --- a/test/mocks/api/BUILD +++ b/test/mocks/api/BUILD @@ -15,6 +15,7 @@ envoy_cc_mock( deps = [ "//include/envoy/api:api_interface", "//include/envoy/api:os_sys_calls_interface", + "//source/common/api:os_sys_calls_lib", "//test/mocks/filesystem:filesystem_mocks", ], ) diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index 0d15602e8974..42622f4292be 100644 --- a/test/mocks/api/mocks.h +++ b/test/mocks/api/mocks.h @@ -3,6 +3,8 @@ #include #include +#include "common/api/os_sys_calls_impl.h" + #include "envoy/api/api.h" #include "envoy/api/os_sys_calls.h" #include "envoy/event/dispatcher.h" @@ -34,7 +36,7 @@ class MockApi : public Api { std::shared_ptr file_{new Filesystem::MockFile()}; }; -class MockOsSysCalls : public OsSysCalls { +class MockOsSysCalls : public OsSysCallsImpl { public: MockOsSysCalls(); ~MockOsSysCalls(); diff --git a/test/test_common/singleton_injector.h b/test/test_common/singleton_injector.h index b2c45924c4be..7f64ae68c739 100644 --- a/test/test_common/singleton_injector.h +++ b/test/test_common/singleton_injector.h @@ -6,14 +6,14 @@ namespace Envoy { template class TestThreadsafeSingletonInjector { public: TestThreadsafeSingletonInjector(T* instance) { - latched_instance_ = ThreadSafeSingleton::get(); + latched_instance_ = &ThreadSafeSingleton::get(); ThreadSafeSingleton::instance_ = instance; } ~TestThreadsafeSingletonInjector() { ThreadSafeSingleton::instance_ = latched_instance_; } private: - static T* latched_instance_; + T* latched_instance_; }; } // namespace Envoy From c21aa9b19ba508ce77e8ae2ae29972820ea0791f Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 19 Oct 2017 17:00:27 -0400 Subject: [PATCH 05/10] fix format Signed-off-by: Alyssa Wilk --- source/common/common/singleton.h | 16 ++++++---------- source/server/hot_restart_impl.cc | 6 +++--- test/common/filesystem/filesystem_impl_test.cc | 18 ++++++------------ test/mocks/api/mocks.h | 4 ++-- test/server/hot_restart_impl_test.cc | 3 +-- test/test_common/singleton_injector.h | 7 +++---- 6 files changed, 21 insertions(+), 33 deletions(-) diff --git a/source/common/common/singleton.h b/source/common/common/singleton.h index c2b1d4c35b7d..15b3bb557ea3 100644 --- a/source/common/common/singleton.h +++ b/source/common/common/singleton.h @@ -19,11 +19,9 @@ template class ConstSingleton { } protected: - template friend class TestThreadsafeSingletonInjector; + template friend class TestThreadsafeSingletonInjector; - static void Create() { - instance_ = new T(); - } + static void Create() { instance_ = new T(); } static absl::once_flag create_once_; static T* instance_; @@ -31,17 +29,15 @@ template class ConstSingleton { /* Mutable singleton. All functions in the singleton class *must* be threadsafe.*/ template class ThreadSafeSingleton : public ConstSingleton { - public: +public: static T& get() { absl::call_once(ConstSingleton::create_once_, &ConstSingleton::Create); return *ConstSingleton::instance_; } }; -template -absl::once_flag ConstSingleton::create_once_; +template absl::once_flag ConstSingleton::create_once_; -template -T* ConstSingleton::instance_ = nullptr; +template T* ConstSingleton::instance_ = nullptr; -} // namespace Envoy +} // namespace Envoy diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc index 8e980935c509..4077f8831c88 100644 --- a/source/server/hot_restart_impl.cc +++ b/source/server/hot_restart_impl.cc @@ -109,9 +109,9 @@ std::string SharedMemory::version() { } HotRestartImpl::HotRestartImpl(Options& options) - : options_(options), shmem_(SharedMemory::initialize(options)), - log_lock_(shmem_.log_lock_), access_log_lock_(shmem_.access_log_lock_), - stat_lock_(shmem_.stat_lock_), init_lock_(shmem_.init_lock_) { + : options_(options), shmem_(SharedMemory::initialize(options)), log_lock_(shmem_.log_lock_), + access_log_lock_(shmem_.access_log_lock_), stat_lock_(shmem_.stat_lock_), + init_lock_(shmem_.init_lock_) { my_domain_socket_ = bindDomainSocket(options.restartEpoch()); child_address_ = createDomainSocketAddress((options.restartEpoch() + 1)); if (options.restartEpoch() != 0) { diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index 97fe507f040c..de71c2caaa2e 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -32,8 +32,7 @@ TEST(FileSystemImpl, BadFile) { Thread::MutexBasicLockable lock; Stats::IsolatedStoreImpl store; EXPECT_CALL(dispatcher, createTimer_(_)); - EXPECT_THROW(Filesystem::FileImpl("", dispatcher, lock, store, - std::chrono::milliseconds(10000)), + EXPECT_THROW(Filesystem::FileImpl("", dispatcher, lock, store, std::chrono::milliseconds(10000)), EnvoyException); } @@ -88,8 +87,7 @@ TEST(FileSystemImpl, flushToLogFilePeriodically) { TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, - std::chrono::milliseconds(40)); + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); EXPECT_CALL(os_sys_calls, write_(_, _, _)) @@ -142,8 +140,7 @@ TEST(FileSystemImpl, flushToLogFileOnDemand) { TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, - std::chrono::milliseconds(40)); + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); @@ -217,8 +214,7 @@ TEST(FileSystemImpl, reopenFile) { Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, - std::chrono::milliseconds(40)); + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) .InSequence(sq) @@ -287,8 +283,7 @@ TEST(FilesystemImpl, reopenThrows) { Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, - std::chrono::milliseconds(40)); + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, close(5)).InSequence(sq); EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(-1)); @@ -324,8 +319,7 @@ TEST(FilesystemImpl, bigDataChunkShouldBeFlushedWithoutTimer) { NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, - std::chrono::milliseconds(40)); + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) .WillOnce(Invoke([](int fd, const void* buffer, size_t num_bytes) -> ssize_t { diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index 42622f4292be..46dcacb9c430 100644 --- a/test/mocks/api/mocks.h +++ b/test/mocks/api/mocks.h @@ -3,12 +3,12 @@ #include #include -#include "common/api/os_sys_calls_impl.h" - #include "envoy/api/api.h" #include "envoy/api/os_sys_calls.h" #include "envoy/event/dispatcher.h" +#include "common/api/os_sys_calls_impl.h" + #include "test/mocks/filesystem/mocks.h" #include "gmock/gmock.h" diff --git a/test/server/hot_restart_impl_test.cc b/test/server/hot_restart_impl_test.cc index fca76fe05a29..cb1e9fb504b6 100644 --- a/test/server/hot_restart_impl_test.cc +++ b/test/server/hot_restart_impl_test.cc @@ -1,9 +1,8 @@ +#include "common/api/os_sys_calls_impl.h" #include "common/stats/stats_impl.h" #include "server/hot_restart_impl.h" -#include "common/api/os_sys_calls_impl.h" - #include "test/mocks/api/mocks.h" #include "test/mocks/server/mocks.h" #include "test/test_common/singleton_injector.h" diff --git a/test/test_common/singleton_injector.h b/test/test_common/singleton_injector.h index 7f64ae68c739..344c54c57f8d 100644 --- a/test/test_common/singleton_injector.h +++ b/test/test_common/singleton_injector.h @@ -9,10 +9,9 @@ template class TestThreadsafeSingletonInjector { latched_instance_ = &ThreadSafeSingleton::get(); ThreadSafeSingleton::instance_ = instance; } - ~TestThreadsafeSingletonInjector() { - ThreadSafeSingleton::instance_ = latched_instance_; - } - private: + ~TestThreadsafeSingletonInjector() { ThreadSafeSingleton::instance_ = latched_instance_; } + +private: T* latched_instance_; }; From afd1e6d89ee73fda43cbf4feade1112e608f9dcf Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 31 Oct 2017 11:49:58 -0400 Subject: [PATCH 06/10] splitting out singleton classes, adding unit tests Signed-off-by: Alyssa Wilk --- source/common/common/singleton.h | 26 ++++---- test/common/common/BUILD | 10 +++ test/common/common/singleton_test.cc | 94 ++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 13 deletions(-) create mode 100644 test/common/common/singleton_test.cc diff --git a/source/common/common/singleton.h b/source/common/common/singleton.h index 15b3bb557ea3..f6a1752301f9 100644 --- a/source/common/common/singleton.h +++ b/source/common/common/singleton.h @@ -14,8 +14,17 @@ template class ConstSingleton { * @return const T& a reference to the singleton for class T. */ static const T& get() { - absl::call_once(create_once_, &ConstSingleton::Create); - return *instance_; + static T* instance = new T(); + return *instance; + } +}; + +/* Mutable singleton. All functions in the singleton class *must* be threadsafe.*/ +template class ThreadSafeSingleton { +public: + static T& get() { + absl::call_once(ThreadSafeSingleton::create_once_, &ThreadSafeSingleton::Create); + return *ThreadSafeSingleton::instance_; } protected: @@ -27,17 +36,8 @@ template class ConstSingleton { static T* instance_; }; -/* Mutable singleton. All functions in the singleton class *must* be threadsafe.*/ -template class ThreadSafeSingleton : public ConstSingleton { -public: - static T& get() { - absl::call_once(ConstSingleton::create_once_, &ConstSingleton::Create); - return *ConstSingleton::instance_; - } -}; - -template absl::once_flag ConstSingleton::create_once_; +template absl::once_flag ThreadSafeSingleton::create_once_; -template T* ConstSingleton::instance_ = nullptr; +template T* ThreadSafeSingleton::instance_ = nullptr; } // namespace Envoy diff --git a/test/common/common/BUILD b/test/common/common/BUILD index 6e6b38900882..fab90df01e20 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -46,6 +46,16 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "singleton_test", + srcs = ["singleton_test.cc"], + deps = [ + "//source/common/common:singleton", + "//source/common/common:thread_lib", + "//test/test_common:singleton_injector_lib", + ], +) + envoy_cc_test( name = "utility_test", srcs = ["utility_test.cc"], diff --git a/test/common/common/singleton_test.cc b/test/common/common/singleton_test.cc new file mode 100644 index 000000000000..17fad182134b --- /dev/null +++ b/test/common/common/singleton_test.cc @@ -0,0 +1,94 @@ +#include + +#include "common/common/singleton.h" +#include "common/common/thread.h" + +#include "test/test_common/singleton_injector.h" + +#include "gtest/gtest.h" + +namespace Envoy { + +class TestSingleton { +public: + virtual ~TestSingleton() {} + + virtual void addOne() { + std::unique_lock lock(lock_); + ++value_; + } + + virtual int value() { + std::unique_lock lock(lock_); + return value_; + } + +protected: + std::mutex lock_; + int value_{0}; +}; + +class EvilMathSingleton : public TestSingleton { +public: + EvilMathSingleton() { value_ = -50; } + virtual void addOne() { + std::unique_lock lock(lock_); + ++value_; + ++value_; + } +}; + +class AddTen { +public: + AddTen() { + thread_.reset(new Thread::Thread([this]() -> void { threadRoutine(); })); + } + ~AddTen() { + thread_->join(); + thread_.reset(); + } + +private: + void threadRoutine() { + auto& singleton = ThreadSafeSingleton::get(); + for (int i = 0; i < 10; ++i) { + singleton.addOne(); + } + } + Thread::ThreadPtr thread_; +}; + +TEST(ThreadSafeSingleton, BasicCreationAndMutation) { + auto& singleton = ThreadSafeSingleton::get(); + EXPECT_EQ(&singleton, &ThreadSafeSingleton::get()); + EXPECT_EQ(0, singleton.value()); + singleton.addOne(); + EXPECT_EQ(1, singleton.value()); + + { + AddTen ten; + AddTen twenty; + AddTen thirty; + } + EXPECT_EQ(31, singleton.value()); +} + +TEST(ThreadSafeSingleton, Injection) { + EvilMathSingleton evil_singleton; + + // Sanity check that other tests didn't cause the main singleton to overflow. + int latched_value = ThreadSafeSingleton::get().value(); + ASSERT_GE(latched_value, 0); + + { + TestThreadsafeSingletonInjector override(&evil_singleton); + auto& evil_math_reference = ThreadSafeSingleton::get(); + EXPECT_NE(latched_value, evil_math_reference.value()); + EXPECT_EQ(-50, evil_math_reference.value()); + evil_math_reference.addOne(); + EXPECT_EQ(-48, evil_math_reference.value()); + } + EXPECT_EQ(latched_value, ThreadSafeSingleton::get().value()); +} + +} // namespace Envoy From 57156626703033a4e5ab583e89f96e6350f846e3 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 31 Oct 2017 13:05:19 -0400 Subject: [PATCH 07/10] abseil -> com_google_absl Signed-off-by: Alyssa Wilk --- bazel/repositories.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 8019167fc4da..5560de1cb107 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -183,13 +183,13 @@ def envoy_api_deps(skip_targets): def abseil_deps(skip_targets): if 'abseil' not in skip_targets: native.git_repository( - name = "abseil", + name = "com_google_absl", remote = REPO_LOCATIONS["abseil_cpp"], commit = "6de53819a7173bd446156237a37f53464b7732cc", ) native.bind( name = "abseil_base", - actual = "@abseil//absl/base:base", + actual = "@com_google_absl//absl/base:base", ) def envoy_dependencies(path = "@envoy_deps//", skip_com_google_protobuf = False, skip_targets = [], From 674c3e968b2d537706d98fcc1a5d216d5a81f0ec Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 2 Nov 2017 12:56:26 -0400 Subject: [PATCH 08/10] Moving singletons to source/common/singleton, adding detailed class comments Signed-off-by: Alyssa Wilk --- source/common/api/BUILD | 2 +- source/common/api/os_sys_calls_impl.h | 2 +- source/common/common/BUILD | 6 --- source/common/common/singleton.h | 43 ------------------ source/common/config/BUILD | 8 ++-- source/common/config/metadata.h | 2 +- source/common/config/resources.h | 2 +- source/common/config/utility.h | 2 +- source/common/config/well_known_names.h | 2 +- source/common/http/BUILD | 4 +- source/common/http/header_map_impl.cc | 2 +- source/common/http/headers.h | 2 +- source/common/mongo/BUILD | 2 +- source/common/mongo/proxy.h | 2 +- source/common/ratelimit/ratelimit_impl.h | 2 +- source/common/singleton/BUILD | 11 +++++ source/common/singleton/const_singleton.h | 23 ++++++++++ .../common/singleton/threadsafe_singleton.h | 44 +++++++++++++++++++ source/common/stats/BUILD | 2 +- source/common/stats/stats_impl.h | 1 - source/common/tracing/zipkin/BUILD | 2 +- .../tracing/zipkin/zipkin_core_constants.h | 2 +- .../tracing/zipkin/zipkin_json_field_names.h | 2 +- test/common/common/BUILD | 10 ----- test/common/filesystem/BUILD | 2 +- .../common/filesystem/filesystem_impl_test.cc | 2 +- test/common/singleton/BUILD | 10 +++++ .../threadsafe_singleton_test.cc} | 4 +- test/server/BUILD | 2 +- test/server/hot_restart_impl_test.cc | 2 +- test/test_common/BUILD | 6 +-- ...ctor.h => threadsafe_singleton_injector.h} | 2 +- 32 files changed, 119 insertions(+), 91 deletions(-) delete mode 100644 source/common/common/singleton.h create mode 100644 source/common/singleton/const_singleton.h create mode 100644 source/common/singleton/threadsafe_singleton.h rename test/common/{common/singleton_test.cc => singleton/threadsafe_singleton_test.cc} (94%) rename test/test_common/{singleton_injector.h => threadsafe_singleton_injector.h} (90%) diff --git a/source/common/api/BUILD b/source/common/api/BUILD index 07db89a5e033..31b3d4c61eeb 100644 --- a/source/common/api/BUILD +++ b/source/common/api/BUILD @@ -26,6 +26,6 @@ envoy_cc_library( hdrs = ["os_sys_calls_impl.h"], deps = [ "//include/envoy/api:os_sys_calls_interface", - "//source/common/common:singleton", + "//source/common/singleton:threadsafe_singleton", ], ) diff --git a/source/common/api/os_sys_calls_impl.h b/source/common/api/os_sys_calls_impl.h index 8a1abeab3d7a..b6cdc5639428 100644 --- a/source/common/api/os_sys_calls_impl.h +++ b/source/common/api/os_sys_calls_impl.h @@ -2,7 +2,7 @@ #include "envoy/api/os_sys_calls.h" -#include "common/common/singleton.h" +#include "common/singleton/threadsafe_singleton.h" namespace Envoy { namespace Api { diff --git a/source/common/common/BUILD b/source/common/common/BUILD index f3f3ecea1236..55048c07e20e 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -92,12 +92,6 @@ envoy_cc_library( hdrs = ["non_copyable.h"], ) -envoy_cc_library( - name = "singleton", - hdrs = ["singleton.h"], - external_deps = ["abseil_base"], -) - envoy_cc_library( name = "stl_helpers", hdrs = ["stl_helpers.h"], diff --git a/source/common/common/singleton.h b/source/common/common/singleton.h deleted file mode 100644 index f6a1752301f9..000000000000 --- a/source/common/common/singleton.h +++ /dev/null @@ -1,43 +0,0 @@ -#pragma once - -#include "absl/base/call_once.h" - -namespace Envoy { - -/** - * Immutable singleton pattern. See singleton/manager.h for mutable/destroyable singletons. - */ -template class ConstSingleton { -public: - /** - * Obtain an instance of the singleton for class T. - * @return const T& a reference to the singleton for class T. - */ - static const T& get() { - static T* instance = new T(); - return *instance; - } -}; - -/* Mutable singleton. All functions in the singleton class *must* be threadsafe.*/ -template class ThreadSafeSingleton { -public: - static T& get() { - absl::call_once(ThreadSafeSingleton::create_once_, &ThreadSafeSingleton::Create); - return *ThreadSafeSingleton::instance_; - } - -protected: - template friend class TestThreadsafeSingletonInjector; - - static void Create() { instance_ = new T(); } - - static absl::once_flag create_once_; - static T* instance_; -}; - -template absl::once_flag ThreadSafeSingleton::create_once_; - -template T* ThreadSafeSingleton::instance_ = nullptr; - -} // namespace Envoy diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 92a801aa91a3..12accb1f98e1 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -202,8 +202,8 @@ envoy_cc_library( hdrs = ["metadata.h"], external_deps = ["envoy_base"], deps = [ - "//source/common/common:singleton", "//source/common/protobuf", + "//source/common/singleton:const_singleton", ], ) @@ -221,7 +221,7 @@ envoy_cc_library( envoy_cc_library( name = "resources_lib", hdrs = ["resources.h"], - deps = ["//source/common/common:singleton"], + deps = ["//source/common/singleton:const_singleton"], ) envoy_cc_library( @@ -292,11 +292,11 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:hash_lib", "//source/common/common:hex_lib", - "//source/common/common:singleton", "//source/common/grpc:common_lib", "//source/common/json:config_schemas_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "//source/common/singleton:const_singleton", "//source/common/stats:stats_lib", ], ) @@ -305,5 +305,5 @@ envoy_cc_library( name = "well_known_names", srcs = ["well_known_names.cc"], hdrs = ["well_known_names.h"], - deps = ["//source/common/common:singleton"], + deps = ["//source/common/singleton:const_singleton"], ) diff --git a/source/common/config/metadata.h b/source/common/config/metadata.h index 3eba46565251..b77ffb7f2cc7 100644 --- a/source/common/config/metadata.h +++ b/source/common/config/metadata.h @@ -2,8 +2,8 @@ #include -#include "common/common/singleton.h" #include "common/protobuf/protobuf.h" +#include "common/singleton/const_singleton.h" #include "api/base.pb.h" diff --git a/source/common/config/resources.h b/source/common/config/resources.h index ae8fff8d1715..03f0c9a2efd8 100644 --- a/source/common/config/resources.h +++ b/source/common/config/resources.h @@ -2,7 +2,7 @@ #include -#include "common/common/singleton.h" +#include "common/singleton/const_singleton.h" namespace Envoy { namespace Config { diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 64a6c2c8df61..3a91b1ea818e 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -11,10 +11,10 @@ #include "common/common/assert.h" #include "common/common/hash.h" #include "common/common/hex.h" -#include "common/common/singleton.h" #include "common/grpc/common.h" #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" +#include "common/singleton/const_singleton.h" #include "api/base.pb.h" #include "api/cds.pb.h" diff --git a/source/common/config/well_known_names.h b/source/common/config/well_known_names.h index a277aebc4bd7..6b12c1748e33 100644 --- a/source/common/config/well_known_names.h +++ b/source/common/config/well_known_names.h @@ -6,7 +6,7 @@ #include "envoy/common/exception.h" -#include "common/common/singleton.h" +#include "common/singleton/const_singleton.h" #include "fmt/format.h" diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 27de0dbfa8c1..f5083d04471e 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -166,8 +166,8 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:empty_string", "//source/common/common:non_copyable", - "//source/common/common:singleton", "//source/common/common:utility_lib", + "//source/common/singleton:const_singleton", ], ) @@ -176,7 +176,7 @@ envoy_cc_library( hdrs = ["headers.h"], deps = [ "//include/envoy/http:header_map_interface", - "//source/common/common:singleton", + "//source/common/singleton:const_singleton", ], ) diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index d182d615306f..8a593be8e370 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -6,8 +6,8 @@ #include "common/common/assert.h" #include "common/common/empty_string.h" -#include "common/common/singleton.h" #include "common/common/utility.h" +#include "common/singleton/const_singleton.h" namespace Envoy { namespace Http { diff --git a/source/common/http/headers.h b/source/common/http/headers.h index 905b39878b21..e427803a6323 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -4,7 +4,7 @@ #include "envoy/http/header_map.h" -#include "common/common/singleton.h" +#include "common/singleton/const_singleton.h" namespace Envoy { namespace Http { diff --git a/source/common/mongo/BUILD b/source/common/mongo/BUILD index 1755742687a7..5cd548cf277a 100644 --- a/source/common/mongo/BUILD +++ b/source/common/mongo/BUILD @@ -57,11 +57,11 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:logger_lib", - "//source/common/common:singleton", "//source/common/common:utility_lib", "//source/common/config:filter_json_lib", "//source/common/network:filter_lib", "//source/common/protobuf:utility_lib", + "//source/common/singleton:const_singleton", ], ) diff --git a/source/common/mongo/proxy.h b/source/common/mongo/proxy.h index 10516ac8c86a..523aad249344 100644 --- a/source/common/mongo/proxy.h +++ b/source/common/mongo/proxy.h @@ -19,10 +19,10 @@ #include "common/buffer/buffer_impl.h" #include "common/common/logger.h" -#include "common/common/singleton.h" #include "common/mongo/utility.h" #include "common/network/filter_impl.h" #include "common/protobuf/utility.h" +#include "common/singleton/const_singleton.h" #include "api/filter/network/mongo_proxy.pb.h" diff --git a/source/common/ratelimit/ratelimit_impl.h b/source/common/ratelimit/ratelimit_impl.h index cce5cd04025f..8b5459a923e9 100644 --- a/source/common/ratelimit/ratelimit_impl.h +++ b/source/common/ratelimit/ratelimit_impl.h @@ -10,7 +10,7 @@ #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/cluster_manager.h" -#include "common/common/singleton.h" +#include "common/singleton/const_singleton.h" #include "source/common/ratelimit/ratelimit.pb.h" diff --git a/source/common/singleton/BUILD b/source/common/singleton/BUILD index f11a14b080ec..3dad3f52a689 100644 --- a/source/common/singleton/BUILD +++ b/source/common/singleton/BUILD @@ -8,6 +8,11 @@ load( envoy_package() +envoy_cc_library( + name = "const_singleton", + hdrs = ["const_singleton.h"], +) + envoy_cc_library( name = "manager_impl_lib", srcs = ["manager_impl.cc"], @@ -19,3 +24,9 @@ envoy_cc_library( "//source/common/common:thread_lib", ], ) + +envoy_cc_library( + name = "threadsafe_singleton", + hdrs = ["threadsafe_singleton.h"], + external_deps = ["abseil_base"], +) diff --git a/source/common/singleton/const_singleton.h b/source/common/singleton/const_singleton.h new file mode 100644 index 000000000000..f10603b1e5fa --- /dev/null +++ b/source/common/singleton/const_singleton.h @@ -0,0 +1,23 @@ +#pragma once + +namespace Envoy { + +/** + * ConstSingleton allows easy global cross-thread access to a const object. + * + * This singleton should be used for data which is initialized once at + * start-up and then be treated as immutable const data thereafter. + */ +template class ConstSingleton { +public: + /** + * Obtain an instance of the singleton for class T. + * @return const T& a reference to the singleton for class T. + */ + static const T& get() { + static T* instance = new T(); + return *instance; + } +}; + +} // namespace Envoy diff --git a/source/common/singleton/threadsafe_singleton.h b/source/common/singleton/threadsafe_singleton.h new file mode 100644 index 000000000000..aee90597ec99 --- /dev/null +++ b/source/common/singleton/threadsafe_singleton.h @@ -0,0 +1,44 @@ +#pragma once + +#include "absl/base/call_once.h" + +namespace Envoy { + +/** + * ThreadSafeSingleton allows easy global cross-thread access to a non-const object. + * + * This singleton class should be used for singletons which must be globally + * accessible but can not be marked as const. All functions in the singleton class + * *must* be threadsafe. + * + * Note that there is heavy resistence in Envoy to adding this type of singleton + * if data will persist with state changes across tests, as it becomes difficult + * to write clean unit tests if a state change in one test will persist into + * another test. Be wary of using it. A example of acceptable usage is OsSyscallsImpl, + * where the functions are not strictly speaking const, but affect the OS rather than the + * class itself. An example of unacceptable usage upstream would be for + * globally accessible stat counters, it would have the aforementioned problem + * where state "leaks" across tests. + * + * */ +template class ThreadSafeSingleton { +public: + static T& get() { + absl::call_once(ThreadSafeSingleton::create_once_, &ThreadSafeSingleton::Create); + return *ThreadSafeSingleton::instance_; + } + +protected: + template friend class TestThreadsafeSingletonInjector; + + static void Create() { instance_ = new T(); } + + static absl::once_flag create_once_; + static T* instance_; +}; + +template absl::once_flag ThreadSafeSingleton::create_once_; + +template T* ThreadSafeSingleton::instance_ = nullptr; + +} // namespace Envoy diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD index 8e8a835157b4..d3fc782fa4fb 100644 --- a/source/common/stats/BUILD +++ b/source/common/stats/BUILD @@ -18,10 +18,10 @@ envoy_cc_library( "//include/envoy/server:options_interface", "//include/envoy/stats:stats_interface", "//source/common/common:assert_lib", - "//source/common/common:singleton", "//source/common/common:utility_lib", "//source/common/config:well_known_names", "//source/common/protobuf", + "//source/common/singleton:const_singleton", ], ) diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h index 34e35fe72f46..45ff54e8ba9b 100644 --- a/source/common/stats/stats_impl.h +++ b/source/common/stats/stats_impl.h @@ -15,7 +15,6 @@ #include "envoy/stats/stats.h" #include "common/common/assert.h" -#include "common/common/singleton.h" #include "common/protobuf/protobuf.h" #include "api/bootstrap.pb.h" diff --git a/source/common/tracing/zipkin/BUILD b/source/common/tracing/zipkin/BUILD index 764c3928b9d0..52c1ea52d1dc 100644 --- a/source/common/tracing/zipkin/BUILD +++ b/source/common/tracing/zipkin/BUILD @@ -41,13 +41,13 @@ envoy_cc_library( "//include/envoy/upstream:cluster_manager_interface", "//source/common/common:enum_to_int", "//source/common/common:hex_lib", - "//source/common/common:singleton", "//source/common/common:utility_lib", "//source/common/http:header_map_lib", "//source/common/http:message_lib", "//source/common/http:utility_lib", "//source/common/json:json_loader_lib", "//source/common/network:address_lib", + "//source/common/singleton:const_singleton", "//source/common/tracing:http_tracer_lib", ], ) diff --git a/source/common/tracing/zipkin/zipkin_core_constants.h b/source/common/tracing/zipkin/zipkin_core_constants.h index 61696a26c981..84c0bcea7c66 100644 --- a/source/common/tracing/zipkin/zipkin_core_constants.h +++ b/source/common/tracing/zipkin/zipkin_core_constants.h @@ -2,7 +2,7 @@ #include -#include "common/common/singleton.h" +#include "common/singleton/const_singleton.h" namespace Envoy { namespace Zipkin { diff --git a/source/common/tracing/zipkin/zipkin_json_field_names.h b/source/common/tracing/zipkin/zipkin_json_field_names.h index 9ffb92886be0..09b841ca3192 100644 --- a/source/common/tracing/zipkin/zipkin_json_field_names.h +++ b/source/common/tracing/zipkin/zipkin_json_field_names.h @@ -2,7 +2,7 @@ #include -#include "common/common/singleton.h" +#include "common/singleton/const_singleton.h" namespace Envoy { namespace Zipkin { diff --git a/test/common/common/BUILD b/test/common/common/BUILD index fab90df01e20..6e6b38900882 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -46,16 +46,6 @@ envoy_cc_test( ], ) -envoy_cc_test( - name = "singleton_test", - srcs = ["singleton_test.cc"], - deps = [ - "//source/common/common:singleton", - "//source/common/common:thread_lib", - "//test/test_common:singleton_injector_lib", - ], -) - envoy_cc_test( name = "utility_test", srcs = ["utility_test.cc"], diff --git a/test/common/filesystem/BUILD b/test/common/filesystem/BUILD index 21749ec136c2..5db2ed03a0cb 100644 --- a/test/common/filesystem/BUILD +++ b/test/common/filesystem/BUILD @@ -22,7 +22,7 @@ envoy_cc_test( "//test/mocks/event:event_mocks", "//test/mocks/filesystem:filesystem_mocks", "//test/test_common:environment_lib", - "//test/test_common:singleton_injector_lib", + "//test/test_common:threadsafe_singleton_injector_lib", ], ) diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index de71c2caaa2e..a59d0fa44fdc 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -11,7 +11,7 @@ #include "test/mocks/event/mocks.h" #include "test/mocks/filesystem/mocks.h" #include "test/test_common/environment.h" -#include "test/test_common/singleton_injector.h" +#include "test/test_common/threadsafe_singleton_injector.h" #include "gmock/gmock.h" #include "gtest/gtest.h" diff --git a/test/common/singleton/BUILD b/test/common/singleton/BUILD index 629da8f04ce1..dd1422d9370c 100644 --- a/test/common/singleton/BUILD +++ b/test/common/singleton/BUILD @@ -15,3 +15,13 @@ envoy_cc_test( "//source/common/singleton:manager_impl_lib", ], ) + +envoy_cc_test( + name = "threadsafe_singleton_test", + srcs = ["threadsafe_singleton_test.cc"], + deps = [ + "//source/common/common:thread_lib", + "//source/common/singleton:threadsafe_singleton", + "//test/test_common:threadsafe_singleton_injector_lib", + ], +) diff --git a/test/common/common/singleton_test.cc b/test/common/singleton/threadsafe_singleton_test.cc similarity index 94% rename from test/common/common/singleton_test.cc rename to test/common/singleton/threadsafe_singleton_test.cc index 17fad182134b..12011b7fae8c 100644 --- a/test/common/common/singleton_test.cc +++ b/test/common/singleton/threadsafe_singleton_test.cc @@ -1,9 +1,9 @@ #include -#include "common/common/singleton.h" #include "common/common/thread.h" +#include "common/singleton/threadsafe_singleton.h" -#include "test/test_common/singleton_injector.h" +#include "test/test_common/threadsafe_singleton_injector.h" #include "gtest/gtest.h" diff --git a/test/server/BUILD b/test/server/BUILD index 8d908cee0f8f..f1204e3b5ae8 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -63,7 +63,7 @@ envoy_cc_test( "//source/common/stats:stats_lib", "//source/server:hot_restart_lib", "//test/mocks/server:server_mocks", - "//test/test_common:singleton_injector_lib", + "//test/test_common:threadsafe_singleton_injector_lib", ], ) diff --git a/test/server/hot_restart_impl_test.cc b/test/server/hot_restart_impl_test.cc index cb1e9fb504b6..fe6f4d0efa39 100644 --- a/test/server/hot_restart_impl_test.cc +++ b/test/server/hot_restart_impl_test.cc @@ -5,7 +5,7 @@ #include "test/mocks/api/mocks.h" #include "test/mocks/server/mocks.h" -#include "test/test_common/singleton_injector.h" +#include "test/test_common/threadsafe_singleton_injector.h" #include "gtest/gtest.h" diff --git a/test/test_common/BUILD b/test/test_common/BUILD index 87084c557f46..f467c8a19108 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -92,9 +92,9 @@ envoy_cc_test_library( ) envoy_cc_library( - name = "singleton_injector_lib", - hdrs = ["singleton_injector.h"], + name = "threadsafe_singleton_injector_lib", + hdrs = ["threadsafe_singleton_injector.h"], deps = [ - "//source/common/common:singleton", + "//source/common/singleton:threadsafe_singleton", ], ) diff --git a/test/test_common/singleton_injector.h b/test/test_common/threadsafe_singleton_injector.h similarity index 90% rename from test/test_common/singleton_injector.h rename to test/test_common/threadsafe_singleton_injector.h index 344c54c57f8d..d4ca322e02e5 100644 --- a/test/test_common/singleton_injector.h +++ b/test/test_common/threadsafe_singleton_injector.h @@ -1,4 +1,4 @@ -#include "common/common/singleton.h" +#include "common/singleton/threadsafe_singleton.h" namespace Envoy { From fea701e3512fd086cb00405911799bf38a30966e Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 9 Nov 2017 14:33:49 -0500 Subject: [PATCH 09/10] bowing to the cult of pedagogy ("how many spaces after a period" -> https://www.cultofpedagogy.com/two-spaces-after-period/) Signed-off-by: Alyssa Wilk --- source/common/singleton/threadsafe_singleton.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/singleton/threadsafe_singleton.h b/source/common/singleton/threadsafe_singleton.h index aee90597ec99..2786a195f185 100644 --- a/source/common/singleton/threadsafe_singleton.h +++ b/source/common/singleton/threadsafe_singleton.h @@ -14,9 +14,9 @@ namespace Envoy { * Note that there is heavy resistence in Envoy to adding this type of singleton * if data will persist with state changes across tests, as it becomes difficult * to write clean unit tests if a state change in one test will persist into - * another test. Be wary of using it. A example of acceptable usage is OsSyscallsImpl, + * another test. Be wary of using it. A example of acceptable usage is OsSyscallsImpl, * where the functions are not strictly speaking const, but affect the OS rather than the - * class itself. An example of unacceptable usage upstream would be for + * class itself. An example of unacceptable usage upstream would be for * globally accessible stat counters, it would have the aforementioned problem * where state "leaks" across tests. * From cd31966b580c57bbc48d2b61d30bfa29ba8fd3f7 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 20 Nov 2017 12:15:53 -0500 Subject: [PATCH 10/10] renaming abseil per declared workspace name (thanks jmillikin) Signed-off-by: Alyssa Wilk --- bazel/repositories.bzl | 8 ++++---- bazel/repository_locations.bzl | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 0586bac5178e..c2a9d3925434 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -232,7 +232,7 @@ def envoy_dependencies(path = "@envoy_deps//", skip_targets = []): # The long repo names (`com_github_fmtlib_fmt` instead of `fmtlib`) are # semi-standard in the Bazel community, intended to avoid both duplicate # dependencies and name conflicts. - _com_github_abseil_abseil_cpp() + _com_google_absl() _com_github_bombela_backward() _com_github_cyan4973_xxhash() _com_github_eile_tclap() @@ -359,11 +359,11 @@ def _com_google_googletest(): actual = "@com_google_googletest//:gtest", ) -def _com_github_abseil_abseil_cpp(): - _repository_impl("com_github_abseil_abseil_cpp") +def _com_google_absl(): + _repository_impl("com_google_absl") native.bind( name = "abseil_base", - actual = "@com_github_abseil_abseil_cpp//absl/base:base", + actual = "@com_google_absl//absl/base:base", ) def _com_google_protobuf(): diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 37abf3dadf7f..efa570258910 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -1,5 +1,5 @@ REPOSITORY_LOCATIONS = dict( - com_github_abseil_abseil_cpp = dict( + com_google_absl = dict( commit = "6de53819a7173bd446156237a37f53464b7732cc", remote = "https://github.com/abseil/abseil-cpp", ),