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

build: adding an disable_exceptions (which does not yet work) #27811

Merged
merged 5 commits into from
Jun 7, 2023
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
7 changes: 7 additions & 0 deletions bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,13 @@ config_setting(
values = {"define": "envoy_yaml=disabled"},
)

# The goal here is to allow Envoy to build with this option but it is not yet
# complete. See https://github.com/envoyproxy/envoy/issues/27412
config_setting(
name = "disable_exceptions",
values = {"define": "envoy_exceptions=disabled"},
)

config_setting(
name = "disable_envoy_mobile_listener",
values = {"define": "envoy_mobile_listener=disabled"},
Expand Down
2 changes: 2 additions & 0 deletions bazel/envoy_build_system.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ load(
_envoy_select_admin_html = "envoy_select_admin_html",
_envoy_select_admin_no_html = "envoy_select_admin_no_html",
_envoy_select_boringssl = "envoy_select_boringssl",
_envoy_select_disable_exceptions = "envoy_select_disable_exceptions",
_envoy_select_disable_logging = "envoy_select_disable_logging",
_envoy_select_enable_http3 = "envoy_select_enable_http3",
_envoy_select_enable_http_datagrams = "envoy_select_enable_http_datagrams",
Expand Down Expand Up @@ -242,6 +243,7 @@ envoy_select_disable_logging = _envoy_select_disable_logging
envoy_select_google_grpc = _envoy_select_google_grpc
envoy_select_enable_http3 = _envoy_select_enable_http3
envoy_select_enable_yaml = _envoy_select_enable_yaml
envoy_select_disable_exceptions = _envoy_select_disable_exceptions
envoy_select_hot_restart = _envoy_select_hot_restart
envoy_select_enable_http_datagrams = _envoy_select_enable_http_datagrams
envoy_select_signal_trace = _envoy_select_signal_trace
Expand Down
3 changes: 2 additions & 1 deletion bazel/envoy_internal.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# DO NOT LOAD THIS FILE. Targets from this file should be considered private
# and not used outside of the @envoy//bazel package.
load(":envoy_select.bzl", "envoy_select_admin_html", "envoy_select_disable_logging", "envoy_select_google_grpc", "envoy_select_hot_restart", "envoy_select_signal_trace", "envoy_select_static_extension_registration")
load(":envoy_select.bzl", "envoy_select_admin_html", "envoy_select_disable_exceptions", "envoy_select_disable_logging", "envoy_select_google_grpc", "envoy_select_hot_restart", "envoy_select_signal_trace", "envoy_select_static_extension_registration")

# Compute the final copts based on various options.
def envoy_copts(repository, test = False):
Expand Down Expand Up @@ -119,6 +119,7 @@ def envoy_copts(repository, test = False):
repository + "//bazel:uhv_enabled": ["-DENVOY_ENABLE_UHV"],
"//conditions:default": [],
}) + envoy_select_hot_restart(["-DENVOY_HOT_RESTART"], repository) + \
envoy_select_disable_exceptions(["-fno-unwind-tables", "-fno-exceptions"], repository) + \
envoy_select_admin_html(["-DENVOY_ADMIN_HTML"], repository) + \
envoy_select_static_extension_registration(["-DENVOY_STATIC_EXTENSION_REGISTRATION"], repository) + \
envoy_select_disable_logging(["-DENVOY_DISABLE_LOGGING"], repository) + \
Expand Down
3 changes: 2 additions & 1 deletion bazel/envoy_mobile_defines.bzl
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# DO NOT LOAD THIS FILE. Load envoy_build_system.bzl instead.
load(":envoy_select.bzl", "envoy_select_admin_functionality", "envoy_select_enable_http3", "envoy_select_enable_http_datagrams", "envoy_select_enable_yaml", "envoy_select_envoy_mobile_listener", "envoy_select_envoy_mobile_request_compression", "envoy_select_envoy_mobile_stats_reporting", "envoy_select_google_grpc")
load(":envoy_select.bzl", "envoy_select_admin_functionality", "envoy_select_disable_exceptions", "envoy_select_enable_http3", "envoy_select_enable_http_datagrams", "envoy_select_enable_yaml", "envoy_select_envoy_mobile_listener", "envoy_select_envoy_mobile_request_compression", "envoy_select_envoy_mobile_stats_reporting", "envoy_select_google_grpc")

# Compute the defines needed for Envoy Mobile libraries that don't use Envoy's main library wrappers.
def envoy_mobile_defines(repository):
return envoy_select_admin_functionality(["ENVOY_ADMIN_FUNCTIONALITY"], repository) + \
envoy_select_enable_http3(["ENVOY_ENABLE_QUIC"], repository) + \
envoy_select_enable_yaml(["ENVOY_ENABLE_YAML"], repository) + \
envoy_select_disable_exceptions(["ENVOY_DISABLE_EXCEPTIONS"], repository) + \
envoy_select_enable_http_datagrams(["ENVOY_ENABLE_HTTP_DATAGRAMS"], repository) + \
envoy_select_envoy_mobile_listener(["ENVOY_MOBILE_ENABLE_LISTENER"], repository) + \
envoy_select_envoy_mobile_stats_reporting(["ENVOY_MOBILE_STATS_REPORTING"], repository) + \
Expand Down
7 changes: 7 additions & 0 deletions bazel/envoy_select.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ def envoy_select_enable_yaml(xs, repository = ""):
"//conditions:default": xs,
})

# Selects the given values if exceptions are disabled in the current build.
def envoy_select_disable_exceptions(xs, repository = ""):
return select({
repository + "//bazel:disable_exceptions": xs,
"//conditions:default": [],
})

# Selects the given values if HTTP datagram support is enabled in the current build.
def envoy_select_enable_http_datagrams(xs, repository = ""):
return select({
Expand Down
10 changes: 3 additions & 7 deletions mobile/library/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ envoy_status_t Engine::main(std::unique_ptr<Envoy::OptionsImpl>&& options) {
std::unique_ptr<EngineCommon> main_common;
{
Thread::LockGuard lock(mutex_);
try {
TRY_NEEDS_AUDIT {
if (event_tracker_.track != nullptr) {
assert_handler_registration_ =
Assert::addDebugAssertionFailureRecordAction([this](const char* location) {
Expand Down Expand Up @@ -79,13 +79,9 @@ envoy_status_t Engine::main(std::unique_ptr<Envoy::OptionsImpl>&& options) {
event_dispatcher_ = &server_->dispatcher();

cv_.notifyAll();
} catch (const Envoy::NoServingException& e) {
PANIC(e.what());
} catch (const Envoy::MalformedArgvException& e) {
PANIC(e.what());
} catch (const Envoy::EnvoyException& e) {
PANIC(e.what());
}
END_TRY
CATCH(const Envoy::EnvoyException& e, { PANIC(e.what()); });

// Note: We're waiting longer than we might otherwise to drain to the main thread's dispatcher.
// This is because we're not simply waiting for its availability and for it to have started, but
Expand Down
31 changes: 30 additions & 1 deletion source/common/common/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,34 @@ class MainThread {

#define END_TRY }

#ifdef ENVOY_DISABLE_EXCEPTIONS
#define TRY_NEEDS_AUDIT {
#else
// TODO(chaoqinli-1123): Remove this macros after we have removed all the exceptions from data
// plane.
#define TRY_NEEDS_AUDIT try
#define TRY_NEEDS_AUDIT try {
#endif

#ifdef ENVOY_DISABLE_EXCEPTIONS
#define CATCH(ExceptionType, Handler)
#else
#define CATCH(Exception, Handler) \
catch (Exception) { \
Handler \
}
#endif

#ifdef ENVOY_DISABLE_EXCEPTIONS
#define MULTI_CATCH(ExceptionType, Handler)
#else
#define MULTI_CATCH(Exception, Handler, Handler2) \
catch (Exception) { \
Handler \
} \
catch (...) { \
Handler2 \
}
#endif

// These convenience macros assert properties of the threading system, when
// feasible. There is a platform-specific mechanism for determining whether the
Expand Down Expand Up @@ -277,6 +302,9 @@ class MainThread {

#endif

#ifdef ENVOY_DISABLE_EXCEPTIONS
#define TRY_ASSERT_MAIN_THREAD {
#else
/**
* To improve exception safety in data plane, we plan to forbid the use of raw
* try in the core code base. This macros uses main thread assertion to make
Expand All @@ -285,6 +313,7 @@ class MainThread {
#define TRY_ASSERT_MAIN_THREAD \
try { \
ASSERT_IS_MAIN_OR_TEST_THREAD();
#endif

/**
* RAII class to override thread assertions checks in the macros:
Expand Down
5 changes: 3 additions & 2 deletions source/common/filter/config_discovery_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,12 @@ void FilterConfigProviderManagerImplBase::applyLastOrDefaultConfig(
subscription->lastFactoryName());
last_config_valid = true;
}
END_TRY catch (const EnvoyException& e) {
END_TRY CATCH(const EnvoyException& e, {
ENVOY_LOG(debug, "ECDS subscription {} is invalid in a listener context: {}.",
filter_config_name, e.what());
subscription->incrementConflictCounter();
}
});

if (last_config_valid) {
provider.onConfigUpdate(*subscription->lastConfig(), subscription->lastVersionInfo(),
nullptr);
Expand Down
5 changes: 1 addition & 4 deletions source/common/router/header_parser_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ std::string HeaderParser::translateMetadataFormat(const std::string& header_valu
int subs = absl::StrReplaceAll({{matches[0].as_string(), new_format}}, &new_header_value);
ASSERT(subs > 0);
}
END_TRY
catch (Json::Exception& e) {
return header_value;
}
END_TRY CATCH(Json::Exception & e, { return header_value; });
}

return new_header_value;
Expand Down
4 changes: 2 additions & 2 deletions source/common/runtime/runtime_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -690,13 +690,13 @@ SnapshotImplPtr LoaderImpl::createNewSnapshot() {
++disk_layers;
}
END_TRY
catch (EnvoyException& e) {
CATCH(EnvoyException & e, {
// TODO(htuch): Consider latching here, rather than ignoring the
// layer. This would be consistent with filesystem RTDS.
++error_layers;
ENVOY_LOG(debug, "error loading runtime values for layer {} from disk: {}",
layer.DebugString(), e.what());
}
});
}
break;
}
Expand Down
4 changes: 2 additions & 2 deletions source/common/secret/sds_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ void SdsApi::onWatchUpdate() {
}
}
END_TRY
catch (const EnvoyException& e) {
CATCH(const EnvoyException& e, {
ENVOY_LOG_MISC(warn, fmt::format("Failed to reload certificates: {}", e.what()));
sds_api_stats_.key_rotation_failed_.inc();
}
});
}

void SdsApi::onConfigUpdate(const std::vector<Config::DecodedResourceRef>& resources,
Expand Down
5 changes: 2 additions & 3 deletions source/common/stats/tag_extractor_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ namespace {
std::regex parseStdRegex(const std::string& regex) {
TRY_ASSERT_MAIN_THREAD { return std::regex(regex, std::regex::optimize); }
END_TRY
catch (const std::regex_error& e) {
throw EnvoyException(fmt::format("Invalid regex '{}': {}", regex, e.what()));
}
CATCH(const std::regex_error& e,
{ throw EnvoyException(fmt::format("Invalid regex '{}': {}", regex, e.what())); });
}
} // namespace

Expand Down
6 changes: 3 additions & 3 deletions source/common/upstream/cds_api_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ CdsApiHelper::onConfigUpdate(const std::vector<Config::DecodedResourceRef>& adde
}
}
END_TRY
catch (const EnvoyException& e) {
exception_msgs.push_back(fmt::format("{}: {}", cluster.name(), e.what()));
}
CATCH(const EnvoyException& e,
{ exception_msgs.push_back(fmt::format("{}: {}", cluster.name(), e.what())); });
}

for (const auto& resource_name : removed_resources) {
if (cm_.removeCluster(resource_name)) {
any_applied = true;
Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/health_discovery_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,14 @@ void HdsDelegate::onReceiveMessage(
server_context_.messageValidationContext().dynamicValidationVisitor());
}
END_TRY
catch (const ProtoValidationException& ex) {
CATCH(const ProtoValidationException& ex, {
// Increment error count
stats_.errors_.inc();
ENVOY_LOG(warn, "Unable to validate health check specifier: {}", ex.what());

// Do not continue processing message
return;
}
});

// Set response
auto server_response_ms = PROTOBUF_GET_MS_OR_DEFAULT(*message, interval, 1000);
Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1623,15 +1623,15 @@ const Network::Address::InstanceConstSharedPtr
ClusterImplBase::resolveProtoAddress(const envoy::config::core::v3::Address& address) {
TRY_ASSERT_MAIN_THREAD { return Network::Address::resolveProtoAddress(address); }
END_TRY
catch (EnvoyException& e) {
CATCH(EnvoyException & e, {
if (info_->type() == envoy::config::cluster::v3::Cluster::STATIC ||
info_->type() == envoy::config::cluster::v3::Cluster::EDS) {
throw EnvoyException(fmt::format("{}. Consider setting resolver_name or setting cluster type "
"to 'STRICT_DNS' or 'LOGICAL_DNS'",
e.what()));
}
throw e;
}
});
}

void ClusterImplBase::validateEndpointsForZoneAwareRouting(
Expand Down
4 changes: 2 additions & 2 deletions source/exe/stripped_main_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ void StrippedMainBase::configureHotRestarter(Random::RandomGenerator& random_gen
options_.socketMode());
}
END_TRY
catch (Server::HotRestartDomainSocketInUseException& ex) {
CATCH(Server::HotRestartDomainSocketInUseException & ex, {
// No luck, try again.
ENVOY_LOG_MISC(debug, "dynamic base id: {}", ex.what());
}
});
}

if (restarter == nullptr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ FileBasedMetadataAuthenticator::GetMetadata(grpc::string_ref, grpc::string_ref,
std::string header_value = Envoy::Config::DataSource::read(config_.secret_data(), true, api_);
metadata->insert(std::make_pair(header_key, header_prefix + header_value));
}
END_TRY
catch (const EnvoyException& e) {
return grpc::Status(grpc::StatusCode::NOT_FOUND, e.what());
return {grpc::StatusCode::NOT_FOUND, e.what()};
}
return grpc::Status::OK;
}
Expand Down
1 change: 1 addition & 0 deletions source/extensions/network/dns_resolver/cares/dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ void DnsResolverImpl::PendingResolution::finishResolve() {
TRY_NEEDS_AUDIT {
callback_(pending_response_.status_, std::move(pending_response_.address_list_));
}
END_TRY
catch (const EnvoyException& e) {
ENVOY_LOG(critical, "EnvoyException in c-ares callback: {}", e.what());
dispatcher_.post([s = std::string(e.what())] { throw EnvoyException(s); });
Expand Down
25 changes: 12 additions & 13 deletions source/server/options_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,25 +161,24 @@ OptionsImpl::OptionsImpl(std::vector<std::string> args,
false, "string", cmd);

cmd.setExceptionHandling(false);
TRY_ASSERT_MAIN_THREAD {
cmd.parse(args);
count_ = cmd.getArgList().size();
}
END_TRY
catch (TCLAP::ArgException& e) {

std::function failure_function = [&](TCLAP::ArgException& e) {
TRY_ASSERT_MAIN_THREAD { cmd.getOutput()->failure(cmd, e); }
END_TRY
catch (const TCLAP::ExitException&) {
CATCH(const TCLAP::ExitException&, {
// failure() has already written an informative message to stderr, so all that's left to do
// is throw our own exception with the original message.
throw MalformedArgvException(e.what());
}
}
catch (const TCLAP::ExitException& e) {
// parse() throws an ExitException with status 0 after printing the output for --help and
// --version.
throw NoServingException();
});
};

TRY_ASSERT_MAIN_THREAD {
cmd.parse(args);
count_ = cmd.getArgList().size();
}
END_TRY
MULTI_CATCH(
TCLAP::ArgException & e, { failure_function(e); }, { throw NoServingException(); });

hot_restart_disabled_ = disable_hot_restart.getValue();
mutex_tracing_enabled_ = enable_mutex_tracing.getValue();
Expand Down
Loading