diff --git a/mobile/library/common/BUILD b/mobile/library/common/BUILD index d46174daba41..be1eb95bd9a0 100644 --- a/mobile/library/common/BUILD +++ b/mobile/library/common/BUILD @@ -18,11 +18,9 @@ envoy_cc_library( srcs = [ "engine.cc", "engine.h", - "engine_handle.cc", "main_interface.cc", ], hdrs = [ - "engine_handle.h", "main_interface.h", ], repository = "@envoy", diff --git a/mobile/library/common/engine_handle.cc b/mobile/library/common/engine_handle.cc deleted file mode 100644 index 51287f6daebe..000000000000 --- a/mobile/library/common/engine_handle.cc +++ /dev/null @@ -1,38 +0,0 @@ -#include "library/common/engine_handle.h" - -namespace Envoy { - -envoy_status_t EngineHandle::runOnEngineDispatcher(envoy_engine_t handle, - std::function func) { - if (auto engine = reinterpret_cast(handle)) { - return engine->dispatcher().post([engine, func]() { func(*engine); }); - } - return ENVOY_FAILURE; -} - -envoy_engine_t EngineHandle::initEngine(envoy_engine_callbacks callbacks, envoy_logger logger, - envoy_event_tracker event_tracker) { - auto engine = new Envoy::Engine(callbacks, logger, event_tracker); - return reinterpret_cast(engine); -} - -envoy_status_t EngineHandle::runEngine(envoy_engine_t handle, const char* config, - const char* log_level) { - if (auto engine = reinterpret_cast(handle)) { - engine->run(config, log_level); - return ENVOY_SUCCESS; - } - return ENVOY_FAILURE; -} - -envoy_status_t EngineHandle::terminateEngine(envoy_engine_t handle, bool release) { - auto engine = reinterpret_cast(handle); - envoy_status_t ret = engine->terminate(); - if (release) { - // TODO(jpsim): Always delete engine to avoid leaking it - delete engine; - } - return ret; -} - -} // namespace Envoy diff --git a/mobile/library/common/engine_handle.h b/mobile/library/common/engine_handle.h deleted file mode 100644 index 3b91029e0a3c..000000000000 --- a/mobile/library/common/engine_handle.h +++ /dev/null @@ -1,38 +0,0 @@ -#pragma once - -#include "library/common/engine.h" -#include "library/common/main_interface.h" -#include "library/common/types/c_types.h" - -namespace Envoy { - -/** - * Wrapper class around the singleton engine handle. This allows us to use C++ access modifiers to - * control what functionality dependent code is able to access. Furthermore, this allows C++ access - * to scheduling work on the dispatcher beyond what is available through the public C API. - */ -class EngineHandle { -public: - /** - * Execute function on the dispatcher of the provided engine, if this engine is currently running. - * @param envoy_engine_t, handle to the engine which will be used to execute the function. - * @param func, function that will be executed if engine exists. - * @return bool, true if the function was scheduled on the dispatcher. - */ - static envoy_status_t runOnEngineDispatcher(envoy_engine_t engine, - std::function func); - -private: - static envoy_engine_t initEngine(envoy_engine_callbacks callbacks, envoy_logger logger, - envoy_event_tracker event_tracker); - static envoy_status_t runEngine(envoy_engine_t, const char* config, const char* log_level); - static envoy_status_t terminateEngine(envoy_engine_t handle, bool release); - - // Allow a specific list of functions to access the internal setup/teardown functionality. - friend envoy_engine_t(::init_engine)(envoy_engine_callbacks callbacks, envoy_logger logger, - envoy_event_tracker event_tracker); - friend envoy_status_t(::run_engine)(envoy_engine_t, const char* config, const char* log_level); - friend envoy_status_t(::terminate_engine)(envoy_engine_t engine, bool release); -}; - -} // namespace Envoy diff --git a/mobile/library/common/main_interface.cc b/mobile/library/common/main_interface.cc index 8b280d272ac1..fba50ef1b0a7 100644 --- a/mobile/library/common/main_interface.cc +++ b/mobile/library/common/main_interface.cc @@ -7,13 +7,22 @@ #include "library/common/api/external.h" #include "library/common/data/utility.h" #include "library/common/engine.h" -#include "library/common/engine_handle.h" #include "library/common/extensions/filters/http/platform_bridge/c_types.h" #include "library/common/http/client.h" #include "library/common/network/connectivity_manager.h" // NOLINT(namespace-envoy) +namespace { +envoy_status_t runOnEngineDispatcher(envoy_engine_t handle, + std::function func) { + if (auto engine = reinterpret_cast(handle)) { + return engine->dispatcher().post([engine, func]() { func(*engine); }); + } + return ENVOY_FAILURE; +} +} // namespace + static std::atomic current_stream_handle_{0}; envoy_stream_t init_stream(envoy_engine_t) { return current_stream_handle_++; } @@ -21,7 +30,7 @@ envoy_stream_t init_stream(envoy_engine_t) { return current_stream_handle_++; } envoy_status_t start_stream(envoy_engine_t engine, envoy_stream_t stream, envoy_http_callbacks callbacks, bool explicit_flow_control, uint64_t min_delivery_size) { - return Envoy::EngineHandle::runOnEngineDispatcher( + return runOnEngineDispatcher( engine, [stream, callbacks, explicit_flow_control, min_delivery_size](auto& engine) -> void { engine.httpClient().startStream(stream, callbacks, explicit_flow_control, min_delivery_size); @@ -30,25 +39,22 @@ envoy_status_t start_stream(envoy_engine_t engine, envoy_stream_t stream, envoy_status_t send_headers(envoy_engine_t engine, envoy_stream_t stream, envoy_headers headers, bool end_stream) { - return Envoy::EngineHandle::runOnEngineDispatcher( - engine, ([stream, headers, end_stream](auto& engine) -> void { - engine.httpClient().sendHeaders(stream, headers, end_stream); - })); + return runOnEngineDispatcher(engine, ([stream, headers, end_stream](auto& engine) -> void { + engine.httpClient().sendHeaders(stream, headers, end_stream); + })); } envoy_status_t read_data(envoy_engine_t engine, envoy_stream_t stream, size_t bytes_to_read) { - return Envoy::EngineHandle::runOnEngineDispatcher( - engine, [stream, bytes_to_read](auto& engine) -> void { - engine.httpClient().readData(stream, bytes_to_read); - }); + return runOnEngineDispatcher(engine, [stream, bytes_to_read](auto& engine) -> void { + engine.httpClient().readData(stream, bytes_to_read); + }); } envoy_status_t send_data(envoy_engine_t engine, envoy_stream_t stream, envoy_data data, bool end_stream) { - return Envoy::EngineHandle::runOnEngineDispatcher( - engine, [stream, data, end_stream](auto& engine) -> void { - engine.httpClient().sendData(stream, data, end_stream); - }); + return runOnEngineDispatcher(engine, [stream, data, end_stream](auto& engine) -> void { + engine.httpClient().sendData(stream, data, end_stream); + }); } // TODO: implement. @@ -57,21 +63,20 @@ envoy_status_t send_metadata(envoy_engine_t, envoy_stream_t, envoy_headers) { } envoy_status_t send_trailers(envoy_engine_t engine, envoy_stream_t stream, envoy_headers trailers) { - return Envoy::EngineHandle::runOnEngineDispatcher( - engine, [stream, trailers](auto& engine) -> void { - engine.httpClient().sendTrailers(stream, trailers); - }); + return runOnEngineDispatcher(engine, [stream, trailers](auto& engine) -> void { + engine.httpClient().sendTrailers(stream, trailers); + }); } envoy_status_t reset_stream(envoy_engine_t engine, envoy_stream_t stream) { - return Envoy::EngineHandle::runOnEngineDispatcher( + return runOnEngineDispatcher( engine, [stream](auto& engine) -> void { engine.httpClient().cancelStream(stream); }); } envoy_status_t set_preferred_network(envoy_engine_t engine, envoy_network_t network) { envoy_netconf_t configuration_key = Envoy::Network::ConnectivityManagerImpl::setPreferredNetwork(network); - Envoy::EngineHandle::runOnEngineDispatcher(engine, [configuration_key](auto& engine) -> void { + runOnEngineDispatcher(engine, [configuration_key](auto& engine) -> void { engine.networkConnectivityManager().refreshDns(configuration_key, true); }); // TODO(snowp): Should this return failure ever? @@ -79,7 +84,7 @@ envoy_status_t set_preferred_network(envoy_engine_t engine, envoy_network_t netw } envoy_status_t set_proxy_settings(envoy_engine_t e, const char* host, const uint16_t port) { - return Envoy::EngineHandle::runOnEngineDispatcher( + return runOnEngineDispatcher( e, [proxy_settings = Envoy::Network::ProxySettings::parseHostAndPort(host, port)](auto& engine) -> void { engine.networkConnectivityManager().setProxySettings(proxy_settings); }); @@ -87,20 +92,20 @@ envoy_status_t set_proxy_settings(envoy_engine_t e, const char* host, const uint envoy_status_t record_counter_inc(envoy_engine_t e, const char* elements, envoy_stats_tags tags, uint64_t count) { - return Envoy::EngineHandle::runOnEngineDispatcher( - e, [name = std::string(elements), tags, count](auto& engine) -> void { - engine.recordCounterInc(name, tags, count); - }); + return runOnEngineDispatcher(e, + [name = std::string(elements), tags, count](auto& engine) -> void { + engine.recordCounterInc(name, tags, count); + }); } envoy_status_t dump_stats(envoy_engine_t engine, envoy_data* out) { absl::Notification stats_received; - if (Envoy::EngineHandle::runOnEngineDispatcher( - engine, ([out, &stats_received](auto& engine) -> void { - Envoy::Buffer::OwnedImpl dumped_stats = engine.dumpStats(); - *out = Envoy::Data::Utility::toBridgeData(dumped_stats, 1024 * 1024 * 100); - stats_received.Notify(); - })) == ENVOY_FAILURE) { + if (runOnEngineDispatcher(engine, ([out, &stats_received](auto& engine) -> void { + Envoy::Buffer::OwnedImpl dumped_stats = engine.dumpStats(); + *out = Envoy::Data::Utility::toBridgeData(dumped_stats, + 1024 * 1024 * 100); + stats_received.Notify(); + })) == ENVOY_FAILURE) { return ENVOY_FAILURE; } stats_received.WaitForNotification(); @@ -108,7 +113,7 @@ envoy_status_t dump_stats(envoy_engine_t engine, envoy_data* out) { } void flush_stats(envoy_engine_t e) { - Envoy::EngineHandle::runOnEngineDispatcher(e, [](auto& engine) { engine.flushStats(); }); + runOnEngineDispatcher(e, [](auto& engine) { engine.flushStats(); }); } envoy_status_t register_platform_api(const char* name, void* api) { @@ -118,18 +123,29 @@ envoy_status_t register_platform_api(const char* name, void* api) { envoy_engine_t init_engine(envoy_engine_callbacks callbacks, envoy_logger logger, envoy_event_tracker event_tracker) { - return Envoy::EngineHandle::initEngine(callbacks, logger, event_tracker); + auto engine = new Envoy::Engine(callbacks, logger, event_tracker); + return reinterpret_cast(engine); } -envoy_status_t run_engine(envoy_engine_t engine, const char* config, const char* log_level) { - return Envoy::EngineHandle::runEngine(engine, config, log_level); +envoy_status_t run_engine(envoy_engine_t handle, const char* config, const char* log_level) { + if (auto engine = reinterpret_cast(handle)) { + engine->run(config, log_level); + return ENVOY_SUCCESS; + } + return ENVOY_FAILURE; } -envoy_status_t terminate_engine(envoy_engine_t engine, bool release) { - return Envoy::EngineHandle::terminateEngine(engine, release); +envoy_status_t terminate_engine(envoy_engine_t handle, bool release) { + auto engine = reinterpret_cast(handle); + envoy_status_t ret = engine->terminate(); + if (release) { + // TODO(jpsim): Always delete engine to avoid leaking it + delete engine; + } + return ret; } envoy_status_t reset_connectivity_state(envoy_engine_t e) { - return Envoy::EngineHandle::runOnEngineDispatcher( + return runOnEngineDispatcher( e, [](auto& engine) { engine.networkConnectivityManager().resetConnectivityState(); }); } diff --git a/mobile/test/common/engine_test.cc b/mobile/test/common/engine_test.cc index 1b0715bb2ac1..ae13340c4bec 100644 --- a/mobile/test/common/engine_test.cc +++ b/mobile/test/common/engine_test.cc @@ -2,7 +2,6 @@ #include "gtest/gtest.h" #include "library/cc/engine_builder.h" #include "library/common/engine.h" -#include "library/common/engine_handle.h" #include "library/common/main_interface.h" namespace Envoy { @@ -79,21 +78,15 @@ TEST_F(EngineTest, AccessEngineAfterInitialization) { ASSERT_TRUE(test_context.on_engine_running.WaitForNotificationWithTimeout(absl::Seconds(10))); absl::Notification getClusterManagerInvoked; - // Scheduling on the dispatcher should work, the engine is running. - EXPECT_EQ(ENVOY_SUCCESS, EngineHandle::runOnEngineDispatcher( - handle, [&getClusterManagerInvoked](Envoy::Engine& engine) { - engine.getClusterManager(); - getClusterManagerInvoked.Notify(); - })); - - // Validate that we actually invoked the function. - EXPECT_TRUE(getClusterManagerInvoked.WaitForNotificationWithTimeout(absl::Seconds(1))); + envoy_data stats_data; + // Running engine functions should work because the engine is running + EXPECT_EQ(ENVOY_SUCCESS, dump_stats(handle, &stats_data)); + release_envoy_data(stats_data); engine_->terminate(); // Now that the engine has been shut down, we no longer expect scheduling to work. - EXPECT_EQ(ENVOY_FAILURE, EngineHandle::runOnEngineDispatcher( - handle, [](Envoy::Engine& engine) { engine.getClusterManager(); })); + EXPECT_EQ(ENVOY_FAILURE, dump_stats(handle, &stats_data)); engine_.reset(); } diff --git a/mobile/test/common/integration/base_client_integration_test.cc b/mobile/test/common/integration/base_client_integration_test.cc index 1921f1000723..b91bba6f45f7 100644 --- a/mobile/test/common/integration/base_client_integration_test.cc +++ b/mobile/test/common/integration/base_client_integration_test.cc @@ -10,7 +10,6 @@ #include "library/cc/bridge_utility.h" #include "library/cc/log_level.h" #include "library/common/engine.h" -#include "library/common/engine_handle.h" #include "library/common/http/header_utility.h" #include "spdlog/spdlog.h" @@ -222,16 +221,15 @@ uint64_t BaseClientIntegrationTest::getCounterValue(const std::string& name) { uint64_t counter_value = 0UL; uint64_t* counter_value_ptr = &counter_value; absl::Notification counter_value_set; - EXPECT_EQ(ENVOY_SUCCESS, - EngineHandle::runOnEngineDispatcher( - rawEngine(), [counter_value_ptr, &name, &counter_value_set](Envoy::Engine& engine) { - Stats::CounterSharedPtr counter = - TestUtility::findCounter(engine.getStatsStore(), name); - if (counter != nullptr) { - *counter_value_ptr = counter->value(); - } - counter_value_set.Notify(); - })); + auto engine = reinterpret_cast(rawEngine()); + engine->dispatcher().post([&] { + Stats::CounterSharedPtr counter = TestUtility::findCounter(engine->getStatsStore(), name); + if (counter != nullptr) { + *counter_value_ptr = counter->value(); + } + counter_value_set.Notify(); + }); + EXPECT_TRUE(counter_value_set.WaitForNotificationWithTimeout(absl::Seconds(5))); return counter_value; } @@ -254,16 +252,14 @@ uint64_t BaseClientIntegrationTest::getGaugeValue(const std::string& name) { uint64_t gauge_value = 0UL; uint64_t* gauge_value_ptr = &gauge_value; absl::Notification gauge_value_set; - EXPECT_EQ(ENVOY_SUCCESS, - EngineHandle::runOnEngineDispatcher( - rawEngine(), [gauge_value_ptr, &name, &gauge_value_set](Envoy::Engine& engine) { - Stats::GaugeSharedPtr gauge = - TestUtility::findGauge(engine.getStatsStore(), name); - if (gauge != nullptr) { - *gauge_value_ptr = gauge->value(); - } - gauge_value_set.Notify(); - })); + auto engine = reinterpret_cast(rawEngine()); + engine->dispatcher().post([&] { + Stats::GaugeSharedPtr gauge = TestUtility::findGauge(engine->getStatsStore(), name); + if (gauge != nullptr) { + *gauge_value_ptr = gauge->value(); + } + gauge_value_set.Notify(); + }); EXPECT_TRUE(gauge_value_set.WaitForNotificationWithTimeout(absl::Seconds(5))); return gauge_value; } diff --git a/mobile/test/non_hermetic/gcp_traffic_director_integration_test.cc b/mobile/test/non_hermetic/gcp_traffic_director_integration_test.cc index 5511b3488878..3723a9e70be5 100644 --- a/mobile/test/non_hermetic/gcp_traffic_director_integration_test.cc +++ b/mobile/test/non_hermetic/gcp_traffic_director_integration_test.cc @@ -26,7 +26,6 @@ #include "absl/synchronization/notification.h" #include "gtest/gtest.h" #include "library/common/data/utility.h" -#include "library/common/engine_handle.h" #include "library/common/types/c_types.h" #include "tools/cpp/runfiles/runfiles.h"