IceGrid fixes

zeroc-ice · Oct 17, 2024 · e5b024b · e5b024b
1 parent 3aa9e0c
commit e5b024b
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 54 deletions.
diff --git a/config/PropertyNames.xml b/config/PropertyNames.xml
@@ -159,7 +159,6 @@
         <property name="Warn.Datagrams" languages="cpp,csharp,java" default="0" />
         <property name="Warn.Dispatch" languages="all" default="1" />
         <property name="Warn.Endpoints" languages="all" default="1" />
-        <property name="Warn.UnknownProperties" languages="java,js" default="1" />
         <property name="Warn.UnusedProperties" languages="all" default="0" />
         <property name="CacheMessageBuffers" languages="csharp,java" default="2" />
         <property name="ThreadInterruptSafe" languages="java" />

diff --git a/cpp/include/Ice/Properties.h b/cpp/include/Ice/Properties.h
@@ -200,8 +200,19 @@ namespace Ice
          */
         std::set<std::string> 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 <code><em>key</em>=<em>value</em></code>.
+         * @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<std::map<std::string, std::string>, StringSeq>
+        parseOptions(std::string_view prefix, const StringSeq& options);
+
     private:
-        void parseLine(std::string_view, const StringConverterPtr&);
+        static std::optional<std::pair<std::string, std::string>>
+        parseLine(std::string_view, const StringConverterPtr&);
 
         void loadConfig();
 

diff --git 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<map<string, string>, StringSeq>
+Ice::Properties::parseOptions(string_view prefix, const StringSeq& options)
+{
+    map<string, string> 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<pair<string, string>>
 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
@@ -539,7 +539,7 @@ const PropertyArray PropertyNames::Glacier2Props
     24
 };
 
-const std::array<PropertyArray,14> PropertyNames::validProps =
+const std::array<PropertyArray, 14> PropertyNames::validProps =
 {
     IceProps,
     IceMXProps,

diff --git 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<PropertyArray,14> validProps;
+        static const std::array<PropertyArray, 14> validProps;
     };
 }
 

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 ]);