From 1c97a3dda5d93755c98ad78913853e7b3d226523 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 10 Jan 2018 12:54:29 -0500 Subject: [PATCH 1/4] Sanity checking libevent init in a few places Signed-off-by: Alyssa Wilk --- source/common/event/dispatcher_impl.cc | 9 +++++++-- source/common/event/libevent.cc | 5 +++++ source/common/event/libevent.h | 2 ++ test/integration/integration.cc | 2 ++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 0e208959eec24..cf5915118fffc 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -26,13 +26,18 @@ namespace Envoy { namespace Event { DispatcherImpl::DispatcherImpl() - : DispatcherImpl(Buffer::WatermarkFactoryPtr{new Buffer::WatermarkBufferFactory}) {} + : DispatcherImpl(Buffer::WatermarkFactoryPtr{new Buffer::WatermarkBufferFactory}) { + // The dispatcher won't work as expected if libevent hasn't been configured to use threads. + ASSERT(Libevent::Global::initialized()); +} DispatcherImpl::DispatcherImpl(Buffer::WatermarkFactoryPtr&& factory) : buffer_factory_(std::move(factory)), base_(event_base_new()), deferred_delete_timer_(createTimer([this]() -> void { clearDeferredDeleteList(); })), post_timer_(createTimer([this]() -> void { runPostCallbacks(); })), - current_to_delete_(&to_delete_1_) {} + current_to_delete_(&to_delete_1_) { + ASSERT(Libevent::Global::initialized()); +} DispatcherImpl::~DispatcherImpl() {} diff --git a/source/common/event/libevent.cc b/source/common/event/libevent.cc index fb206ce8bee80..f5b046297b21d 100644 --- a/source/common/event/libevent.cc +++ b/source/common/event/libevent.cc @@ -10,11 +10,16 @@ namespace Envoy { namespace Event { namespace Libevent { +static bool kInitialized = false; + +bool Global::initialized() { return kInitialized; } + void Global::initialize() { evthread_use_pthreads(); // Ignore SIGPIPE and allow errors to propagate through error codes. signal(SIGPIPE, SIG_IGN); + kInitialized = true; } } // namespace Libevent diff --git a/source/common/event/libevent.h b/source/common/event/libevent.h index 6e7d68216ee90..617965e5aaa37 100644 --- a/source/common/event/libevent.h +++ b/source/common/event/libevent.h @@ -31,6 +31,8 @@ namespace Libevent { */ class Global { public: + // Returns true if initialized() has been called. + static bool initialized(); /** * Initialize the library globally. */ diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 7499c0b2112ae..53f6bad87d92a 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -15,6 +15,7 @@ #include "common/buffer/buffer_impl.h" #include "common/common/assert.h" #include "common/event/dispatcher_impl.h" +#include "common/event/libevent.h" #include "common/network/connection_impl.h" #include "common/network/utility.h" #include "common/upstream/upstream_impl.h" @@ -199,6 +200,7 @@ Network::ClientConnectionPtr BaseIntegrationTest::makeClientConnection(uint32_t void BaseIntegrationTest::initialize() { RELEASE_ASSERT(!initialized_); + RELEASE_ASSERT(Event::Libevent::Global::initialized()); initialized_ = true; createUpstreams(); From 3e31b8764898565d807f793b19a740d430a28721 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 10 Jan 2018 13:15:15 -0500 Subject: [PATCH 2/4] reviewer comments Signed-off-by: Alyssa Wilk --- source/common/event/dispatcher_impl.cc | 2 +- source/common/event/libevent.cc | 6 ++---- source/common/event/libevent.h | 7 +++++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index cf5915118fffc..2787b6fb97372 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -28,7 +28,7 @@ namespace Event { DispatcherImpl::DispatcherImpl() : DispatcherImpl(Buffer::WatermarkFactoryPtr{new Buffer::WatermarkBufferFactory}) { // The dispatcher won't work as expected if libevent hasn't been configured to use threads. - ASSERT(Libevent::Global::initialized()); + RELEASE_ASSERT(Libevent::Global::initialized()); } DispatcherImpl::DispatcherImpl(Buffer::WatermarkFactoryPtr&& factory) diff --git a/source/common/event/libevent.cc b/source/common/event/libevent.cc index f5b046297b21d..bb3b47f4e3d2d 100644 --- a/source/common/event/libevent.cc +++ b/source/common/event/libevent.cc @@ -10,16 +10,14 @@ namespace Envoy { namespace Event { namespace Libevent { -static bool kInitialized = false; - -bool Global::initialized() { return kInitialized; } +bool Global::INITIALIZED = false; void Global::initialize() { evthread_use_pthreads(); // Ignore SIGPIPE and allow errors to propagate through error codes. signal(SIGPIPE, SIG_IGN); - kInitialized = true; + INITIALIZED = true; } } // namespace Libevent diff --git a/source/common/event/libevent.h b/source/common/event/libevent.h index 617965e5aaa37..7a2834991be5d 100644 --- a/source/common/event/libevent.h +++ b/source/common/event/libevent.h @@ -31,8 +31,11 @@ namespace Libevent { */ class Global { public: - // Returns true if initialized() has been called. - static bool initialized(); + // True if initialized() has been called. + static bool INITIALIZED; + + static bool initialized() { return INITIALIZED; } + /** * Initialize the library globally. */ From a4f91851bc1badc75e5a4ed871e33a1145adee9b Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 10 Jan 2018 15:20:47 -0500 Subject: [PATCH 3/4] more reviewer comments Signed-off-by: Alyssa Wilk --- source/common/event/dispatcher_impl.cc | 2 +- source/common/event/libevent.cc | 4 ++-- source/common/event/libevent.h | 9 +++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 2787b6fb97372..d609d9bba9e36 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -36,7 +36,7 @@ DispatcherImpl::DispatcherImpl(Buffer::WatermarkFactoryPtr&& factory) deferred_delete_timer_(createTimer([this]() -> void { clearDeferredDeleteList(); })), post_timer_(createTimer([this]() -> void { runPostCallbacks(); })), current_to_delete_(&to_delete_1_) { - ASSERT(Libevent::Global::initialized()); + RELEASE_ASSERT(Libevent::Global::initialized()); } DispatcherImpl::~DispatcherImpl() {} diff --git a/source/common/event/libevent.cc b/source/common/event/libevent.cc index bb3b47f4e3d2d..e12d4b5a1c71f 100644 --- a/source/common/event/libevent.cc +++ b/source/common/event/libevent.cc @@ -10,14 +10,14 @@ namespace Envoy { namespace Event { namespace Libevent { -bool Global::INITIALIZED = false; +bool Global::is_initialized = false; void Global::initialize() { evthread_use_pthreads(); // Ignore SIGPIPE and allow errors to propagate through error codes. signal(SIGPIPE, SIG_IGN); - INITIALIZED = true; + is_initialized = true; } } // namespace Libevent diff --git a/source/common/event/libevent.h b/source/common/event/libevent.h index 7a2834991be5d..52d0edb794f8e 100644 --- a/source/common/event/libevent.h +++ b/source/common/event/libevent.h @@ -31,15 +31,16 @@ namespace Libevent { */ class Global { public: - // True if initialized() has been called. - static bool INITIALIZED; - - static bool initialized() { return INITIALIZED; } + static bool initialized() { return is_initialized; } /** * Initialize the library globally. */ static void initialize(); + +private: + // True if initialized() has been called. + static bool is_initialized; }; typedef CSmartPtr BasePtr; From cde8a1fdbc047d230f07a0c311e2de5c5f6d756d Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 10 Jan 2018 16:49:41 -0500 Subject: [PATCH 4/4] initialized_ Signed-off-by: Alyssa Wilk --- source/common/event/libevent.cc | 4 ++-- source/common/event/libevent.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/common/event/libevent.cc b/source/common/event/libevent.cc index e12d4b5a1c71f..a81fbeb82adc2 100644 --- a/source/common/event/libevent.cc +++ b/source/common/event/libevent.cc @@ -10,14 +10,14 @@ namespace Envoy { namespace Event { namespace Libevent { -bool Global::is_initialized = false; +bool Global::initialized_ = false; void Global::initialize() { evthread_use_pthreads(); // Ignore SIGPIPE and allow errors to propagate through error codes. signal(SIGPIPE, SIG_IGN); - is_initialized = true; + initialized_ = true; } } // namespace Libevent diff --git a/source/common/event/libevent.h b/source/common/event/libevent.h index 52d0edb794f8e..a07a8816e9e2d 100644 --- a/source/common/event/libevent.h +++ b/source/common/event/libevent.h @@ -31,7 +31,7 @@ namespace Libevent { */ class Global { public: - static bool initialized() { return is_initialized; } + static bool initialized() { return initialized_; } /** * Initialize the library globally. @@ -40,7 +40,7 @@ class Global { private: // True if initialized() has been called. - static bool is_initialized; + static bool initialized_; }; typedef CSmartPtr BasePtr;