From e5b024badd0d66b7649132cc2e1426a6eb395001 Mon Sep 17 00:00:00 2001 From: Joe George Date: Thu, 17 Oct 2024 13:10:29 -0400 Subject: [PATCH] IceGrid fixes --- config/PropertyNames.xml | 1 - cpp/include/Ice/Properties.h | 13 ++- cpp/src/Ice/Properties.cpp | 88 ++++++++++++------- cpp/src/Ice/PropertyNames.cpp | 2 +- cpp/src/Ice/PropertyNames.h | 4 +- cpp/src/IceGrid/Activator.cpp | 5 +- cpp/src/IceGrid/IceGridNode.cpp | 4 +- cpp/src/IceGrid/NodeI.cpp | 10 ++- .../java/com/zeroc/Ice/MetricsAdminI.java | 18 ++-- .../java/com/zeroc/Ice/PropertyNames.java | 1 - js/src/Ice/PropertyNames.js | 1 - 11 files changed, 93 insertions(+), 54 deletions(-) diff --git a/config/PropertyNames.xml b/config/PropertyNames.xml index 57bb34114ab..23ecefceab9 100644 --- a/config/PropertyNames.xml +++ b/config/PropertyNames.xml @@ -159,7 +159,6 @@ - diff --git a/cpp/include/Ice/Properties.h b/cpp/include/Ice/Properties.h index 30465ac3a35..71ace25660b 100644 --- a/cpp/include/Ice/Properties.h +++ b/cpp/include/Ice/Properties.h @@ -200,8 +200,19 @@ namespace Ice */ std::set getUnusedProperties(); + /** + * Parse a sequence of options into a map of key value pairs starting with a prefix. The options are expected to + * be of the form key=value. + * @param prefix The prefix to match. + * @param options The options to parse. + * @return A pair containing a map of matched key value pairs and a sequence of unmatched options. + */ + static std::pair, StringSeq> + parseOptions(std::string_view prefix, const StringSeq& options); + private: - void parseLine(std::string_view, const StringConverterPtr&); + static std::optional> + parseLine(std::string_view, const StringConverterPtr&); void loadConfig(); diff --git a/cpp/src/Ice/Properties.cpp b/cpp/src/Ice/Properties.cpp index af07c811c78..deab0d32d68 100644 --- a/cpp/src/Ice/Properties.cpp +++ b/cpp/src/Ice/Properties.cpp @@ -127,7 +127,11 @@ Ice::Properties::Properties(StringSeq& args, const PropertiesPtr& defaults) { s += "=1"; } - parseLine(s.substr(2), 0); + if (auto optionPair = parseLine(s.substr(2), 0); optionPair) + { + auto [key, value] = *optionPair; + setProperty(key, value); + } loadConfigFiles = true; } else @@ -367,33 +371,14 @@ Ice::Properties::getCommandLineOptions() noexcept StringSeq Ice::Properties::parseCommandLineOptions(string_view prefix, const StringSeq& options) { - string pfx = string{prefix}; - if (!pfx.empty() && pfx[pfx.size() - 1] != '.') - { - pfx += '.'; - } - pfx = "--" + pfx; + auto [matched, unmatched] = parseOptions(prefix, options); - StringSeq result; - for (StringSeq::size_type i = 0; i < options.size(); i++) + for (const auto& [key, value] : matched) { - string opt = options[i]; - - if (opt.find(pfx) == 0) - { - if (opt.find('=') == string::npos) - { - opt += "=1"; - } - - parseLine(opt.substr(2), 0); - } - else - { - result.push_back(opt); - } + setProperty(key, value); } - return result; + + return unmatched; } StringSeq @@ -555,7 +540,11 @@ Ice::Properties::load(string_view file) } firstLine = false; } - parseLine(line, stringConverter); + if (auto optionPair = parseLine(line, stringConverter); optionPair) + { + auto [key, value] = *optionPair; + setProperty(key, value); + } } } } @@ -575,7 +564,44 @@ Ice::Properties::getUnusedProperties() return unusedProperties; } -void +pair, StringSeq> +Ice::Properties::parseOptions(string_view prefix, const StringSeq& options) +{ + map matched; + + string pfx = string{prefix}; + if (!pfx.empty() && pfx[pfx.size() - 1] != '.') + { + pfx += '.'; + } + pfx = "--" + pfx; + + StringSeq unmatched; + for (auto opt : options) + { + if (opt.find(pfx) == 0) + { + if (opt.find('=') == string::npos) + { + opt += "=1"; + } + + if (auto optionPair = parseLine(opt.substr(2), 0); optionPair) + { + auto [key, value] = *optionPair; + matched.emplace(key, value); + } + } + else + { + unmatched.emplace_back(opt); + } + } + + return {matched, unmatched}; +} + +optional> Ice::Properties::parseLine(string_view line, const StringConverterPtr& converter) { string key; @@ -738,18 +764,18 @@ Ice::Properties::parseLine(string_view line, const StringConverterPtr& converter if ((state == Key && key.length() != 0) || (state == Value && key.length() == 0)) { getProcessLogger()->warning("invalid config file entry: \"" + string{line} + "\""); - return; + return nullopt; } else if (key.length() == 0) { - return; + return nullopt; } key = UTF8ToNative(key, converter); value = UTF8ToNative(value, converter); - setProperty(key, value); -} + return make_pair(key, value); +}; void Ice::Properties::loadConfig() diff --git a/cpp/src/Ice/PropertyNames.cpp b/cpp/src/Ice/PropertyNames.cpp index 22796c56de1..36e4dcab829 100644 --- a/cpp/src/Ice/PropertyNames.cpp +++ b/cpp/src/Ice/PropertyNames.cpp @@ -539,7 +539,7 @@ const PropertyArray PropertyNames::Glacier2Props 24 }; -const std::array PropertyNames::validProps = +const std::array PropertyNames::validProps = { IceProps, IceMXProps, diff --git a/cpp/src/Ice/PropertyNames.h b/cpp/src/Ice/PropertyNames.h index b120334caa6..ae3296a5c66 100644 --- a/cpp/src/Ice/PropertyNames.h +++ b/cpp/src/Ice/PropertyNames.h @@ -33,7 +33,7 @@ namespace IceInternal const int length; }; - class ICE_API PropertyNames + class PropertyNames { public: static const PropertyArray ProxyProps; @@ -56,7 +56,7 @@ namespace IceInternal static const PropertyArray IceBTProps; static const PropertyArray Glacier2Props; - static const std::array validProps; + static const std::array validProps; }; } diff --git a/cpp/src/IceGrid/Activator.cpp b/cpp/src/IceGrid/Activator.cpp index 5ad4895eb1f..1d2e4d433fe 100644 --- a/cpp/src/IceGrid/Activator.cpp +++ b/cpp/src/IceGrid/Activator.cpp @@ -1144,7 +1144,10 @@ Activator::destroy() // when there's no more processes and when _deactivating is set to // true. // - _thread.join(); + if (_thread.joinable()) + { + _thread.join(); + } assert(_processes.empty()); } diff --git a/cpp/src/IceGrid/IceGridNode.cpp b/cpp/src/IceGrid/IceGridNode.cpp index 99499eac4f8..11efc16f319 100644 --- a/cpp/src/IceGrid/IceGridNode.cpp +++ b/cpp/src/IceGrid/IceGridNode.cpp @@ -628,8 +628,10 @@ NodeService::stop() _activator->shutdown(); _activator->destroy(); } - catch (...) + catch (const std::exception& ex) { + cout << "unexpected exception while shutting down activator" << endl; + cout << ex.what() << endl; assert(false); } } diff --git a/cpp/src/IceGrid/NodeI.cpp b/cpp/src/IceGrid/NodeI.cpp index 2ec82f24ebd..87214a64983 100644 --- a/cpp/src/IceGrid/NodeI.cpp +++ b/cpp/src/IceGrid/NodeI.cpp @@ -96,10 +96,12 @@ NodeI::NodeI( } } - auto p = Ice::createProperties(); - p->parseCommandLineOptions("", overrides); - auto propDict = p->getPropertiesForPrefix(""); - for (const auto& prop : propDict) + auto parsedOptions = Ice::Properties::parseOptions("", overrides); + + // Since we're using an empty prefix, all properties will be matched + assert(parsedOptions.second.empty()); + + for (const auto& prop : parsedOptions.first) { _propertiesOverride.push_back({prop.first, prop.second}); } diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/MetricsAdminI.java b/java/src/Ice/src/main/java/com/zeroc/Ice/MetricsAdminI.java index efdd497d520..7bbc507eacd 100644 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/MetricsAdminI.java +++ b/java/src/Ice/src/main/java/com/zeroc/Ice/MetricsAdminI.java @@ -35,16 +35,14 @@ static void validateProperties(String prefix, Properties properties) { } } - if (!unknownProps.isEmpty() - && properties.getIcePropertyAsInt("Ice.Warn.UnknownProperties") > 0) { - StringBuffer message = new StringBuffer("found unknown IceMX properties for `"); - message.append(prefix.substring(0, prefix.length() - 1)); - message.append("':"); - for (String p : unknownProps) { - message.append("\n "); - message.append(p); - } - Util.getProcessLogger().warning(message.toString()); + if (unknownProps.size() > 0) { + throw new UnknownPropertyException( + "found unknown properties for " + + propertyArray.name() + + ": '" + + prefix + + "'" + + String.join("\n ", unknownProps)); } } diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/PropertyNames.java b/java/src/Ice/src/main/java/com/zeroc/Ice/PropertyNames.java index 4bfa04a7bcd..879333ce633 100644 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/PropertyNames.java +++ b/java/src/Ice/src/main/java/com/zeroc/Ice/PropertyNames.java @@ -155,7 +155,6 @@ final class PropertyNames new Property("Warn.Datagrams", false, "0", false, null), new Property("Warn.Dispatch", false, "1", false, null), new Property("Warn.Endpoints", false, "1", false, null), - new Property("Warn.UnknownProperties", false, "1", false, null), new Property("Warn.UnusedProperties", false, "0", false, null), new Property("CacheMessageBuffers", false, "2", false, null), new Property("ThreadInterruptSafe", false, "", false, null) diff --git a/js/src/Ice/PropertyNames.js b/js/src/Ice/PropertyNames.js index 0a1cd716e35..11c2d7e8b4b 100644 --- a/js/src/Ice/PropertyNames.js +++ b/js/src/Ice/PropertyNames.js @@ -72,7 +72,6 @@ PropertyNames.IceProps = new PropertyArray("Ice", false, [ new Property("Warn.Connections", false, "0", false, null), new Property("Warn.Dispatch", false, "1", false, null), new Property("Warn.Endpoints", false, "1", false, null), - new Property("Warn.UnknownProperties", false, "1", false, null), new Property("Warn.UnusedProperties", false, "0", false, null) ]);